[PATCH] D154596: [RISCV] Fix required features checking with empty string

[Craig Topper via Phabricator via cfe-commits]

craig.topper accepted this revision.
craig.topper 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/D154596/new/

https://reviews.llvm.org/D154596

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

[PATCH] D154596: [RISCV] Fix required features checking with empty string

[Jim Lin via Phabricator via cfe-commits]

Jim added a comment.

In D154596#4499757 , @wangpc wrote:

> In D154596#4499718 , @Jim wrote:
>
>> In D154596#4499647 , @wangpc wrote:
>>
>>> Can you give an example of intrinsic that doesn't require any extra 
>>> extension enabled?
>>
>> Like read/write csr intrinsics that we add for convenient usage doesn't need 
>> any extra extension enabled.
>
> It seems that `zicsr` is related.
> I am not opposed to this change, just out of curiosity. :-)

read/write csr is not a good example for the newest isa spec. In older isa spec 
2.2, it is included in base i extension.
Anyway. just want to add some intrinsics and them generate corresponding 
instruction that is in base i extension.
And it is reasonable if keep required feature string empty should imply no 
features/extensions required.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154596

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

[PATCH] D154688: [clang] Show verify prefix in error messages

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

In D154688#4498398 , @aaron.ballman 
wrote:

> In D154688#4497967 , @tbaeder wrote:
>
>> When passing a different prefix via `-verify=foo`, the error messages now 
>> say "error: 'foo-error' diagnostics seen but not expected", etc.
>>
>> I'm often working in test files where two different prefixes are used and 
>> I'm always confused about which one of the two the error messages are 
>> talking about.
>
> What I'm confused by is that we list the line numbers of the failures, so the 
> prefix only seems like it's helpful in a case where two prefixes use the same 
> message and the same severity on the same line. e.g., `// foo-error 
> {{whatever}} bar-error {{whatever}}`. In the other cases, either the line 
> number is different, or the severity is different, or the message is 
> different which I thought was giving sufficient context.

This is also reported as being on line 4:

  // RUN: %clang_cc1 -fsyntax-only -fdiagnostics-print-source-range-info 
-verify=bar %s 2>&1
  
  
  static_assert(true); // foo \
   // bar-error {{failed}}

which is also the case if line 4 contained another `foo-error {{failed}}` which 
didn't trigger, leaving developers wondering which one it is.


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

https://reviews.llvm.org/D154688

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

[PATCH] D155213: [HIP] Add `-fno-hip-uniform-block`

[Yaxun Liu via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:7226
+  } else {
+Args.claimAllArgs(options::OPT_fhip_uniform_block,
+  options::OPT_fno_hip_uniform_block);

MaskRay wrote:
> yaxunl wrote:
> > MaskRay wrote:
> > > Why is the -Wunused-command-line-argument warning suppressed in non-IsHIP 
> > > mode?
> > Users may want to add these options to clang config file.
> > 
> > Is there a general rule which options should be claimed?
> Options in a configuration file are automatically claimed.
> 
> I don't know a general rule, but we generally don't claim newly introduced 
> options.
I think I should remove the claimAllArgs for this option. It should behave like 
the usual options when not used.


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

https://reviews.llvm.org/D155213

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

[PATCH] D154764: [ASTImporter] Fields are imported first and reordered for correct layout.

[Ding Fei via Phabricator via cfe-commits]

danix800 updated this revision to Diff 540254.
danix800 retitled this revision from "[clang] [ASTImporter]: force recomputing 
ASTRecordLayout when importing" to "[ASTImporter] Fields are imported first and 
reordered for correct layout.".
danix800 edited the summary of this revision.
danix800 set the repository for this revision to rG LLVM Github Monorepo.
danix800 added a comment.

Fields are imported first and reordered for correct layout.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154764

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp

Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -7970,6 +7970,27 @@
 ToDGOther);
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase,
+   ImportFieldsFirstForCorrectRecordLayoutTest) {
+  // UnaryOperator(&) triggers RecordLayout computation, which relies on
+  // correctly imported fields.
+  auto Code =
+  R"(
+  class A {
+int m() {
+  return &((A *)0)->f1 - &((A *)0)->f2;
+}
+int f1;
+int f2;
+  };
+  )";
+  Decl *FromTU = getTuDecl(Code, Lang_CXX11);
+
+  auto *FromF = FirstDeclMatcher().match(
+  FromTU, cxxMethodDecl(hasName("A::m")));
+  Import(FromF, Lang_CXX11);
+}
+
 TEST_P(ASTImporterOptionSpecificTestBase,
ImportRecordWithLayoutRequestingExpr) {
   TranslationUnitDecl *FromTU = getTuDecl(
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -433,6 +433,7 @@
 Decl *From, DeclContext *, DeclContext *);
 Error ImportImplicitMethods(const CXXRecordDecl *From, CXXRecordDecl *To);
 
+Error ImportFieldDeclDefinition(const FieldDecl *From, const FieldDecl *To);
 Expected ImportCastPath(CastExpr *E);
 Expected ImportAPValue(const APValue );
 
@@ -1850,52 +1851,33 @@
   // different values in two distinct translation units.
   ChildErrorHandlingStrategy HandleChildErrors(FromDC);
 
+  auto MightNeedReordering = [](const Decl *D) {
+return isa(D) || isa(D) || isa(D);
+  };
+
+  // Import everything that might need reordering first.
   Error ChildErrors = Error::success();
   for (auto *From : FromDC->decls()) {
+if (!MightNeedReordering(From)) {
+  continue;
+}
 ExpectedDecl ImportedOrErr = import(From);
 
 // If we are in the process of ImportDefinition(...) for a RecordDecl we
 // want to make sure that we are also completing each FieldDecl. There
 // are currently cases where this does not happen and this is correctness
 // fix since operations such as code generation will expect this to be so.
-if (ImportedOrErr) {
-  FieldDecl *FieldFrom = dyn_cast_or_null(From);
-  Decl *ImportedDecl = *ImportedOrErr;
-  FieldDecl *FieldTo = dyn_cast_or_null(ImportedDecl);
-  if (FieldFrom && FieldTo) {
-RecordDecl *FromRecordDecl = nullptr;
-RecordDecl *ToRecordDecl = nullptr;
-// If we have a field that is an ArrayType we need to check if the array
-// element is a RecordDecl and if so we need to import the definition.
-if (FieldFrom->getType()->isArrayType()) {
-  // getBaseElementTypeUnsafe(...) handles multi-dimensonal arrays for us.
-  FromRecordDecl = FieldFrom->getType()->getBaseElementTypeUnsafe()->getAsRecordDecl();
-  ToRecordDecl = FieldTo->getType()->getBaseElementTypeUnsafe()->getAsRecordDecl();
-}
-
-if (!FromRecordDecl || !ToRecordDecl) {
-  const RecordType *RecordFrom =
-  FieldFrom->getType()->getAs();
-  const RecordType *RecordTo = FieldTo->getType()->getAs();
-
-  if (RecordFrom && RecordTo) {
-FromRecordDecl = RecordFrom->getDecl();
-ToRecordDecl = RecordTo->getDecl();
-  }
-}
-
-if (FromRecordDecl && ToRecordDecl) {
-  if (FromRecordDecl->isCompleteDefinition() &&
-  !ToRecordDecl->isCompleteDefinition()) {
-Error Err = ImportDefinition(FromRecordDecl, ToRecordDecl);
-HandleChildErrors.handleChildImportResult(ChildErrors,
-  std::move(Err));
-  }
-}
-  }
-} else {
+if (!ImportedOrErr) {
   HandleChildErrors.handleChildImportResult(ChildErrors,
 ImportedOrErr.takeError());
+  continue;
+}
+FieldDecl *FieldFrom = dyn_cast_or_null(From);
+Decl *ImportedDecl = *ImportedOrErr;
+FieldDecl *FieldTo = dyn_cast_or_null(ImportedDecl);
+if (FieldFrom && FieldTo) {
+  Error Err = ImportFieldDeclDefinition(FieldFrom, FieldTo);
+  

[PATCH] D155145: Add AVX-VNNI-INT16 instructions.

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added inline comments.



Comment at: llvm/test/CodeGen/X86/avxvnniint16-intrinsics.ll:3
+; RUN: llc < %s -verify-machineinstrs -mtriple=x86_64-unknown-unknown 
--show-mc-encoding -mattr=+avx2,+avxvnniint16 | FileCheck %s 
--check-prefixes=X64
+; RUN: llc < %s -verify-machineinstrs -mtriple=i686-unknown-unknown 
--show-mc-encoding -mattr=+avx2,+avxvnniint16 | FileCheck %s 
--check-prefixes=X86
+

craig.topper wrote:
> pengfei wrote:
> > `X86,CHECK`
> I thought the common prefix had to be first? But I might be wrong
You are right 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155145

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

[PATCH] D154596: [RISCV] Fix required features checking with empty string

[Wang Pengcheng via Phabricator via cfe-commits]

wangpc added a comment.

In D154596#4499718 , @Jim wrote:

> In D154596#4499647 , @wangpc wrote:
>
>> Can you give an example of intrinsic that doesn't require any extra 
>> extension enabled?
>
> Like read/write csr intrinsics that we add for convenient usage doesn't need 
> any extra extension enabled.

It seems that `zicsr` is related.
I am not opposed to this change, just out of curiosity. :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154596

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

[PATCH] D155145: Add AVX-VNNI-INT16 instructions.

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: llvm/lib/Target/X86/X86InstrSSE.td:8303
+  def rr  : I("int_x86_avx2_"#OpcodeStr#"_128")
+  VR128:$src1, VR128:$src2, VR128:$src3)))]>,

This needs to be indented 1 character more so that it looks nested under the 
`set`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155145

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

[PATCH] D155145: Add AVX-VNNI-INT16 instructions.

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: llvm/test/CodeGen/X86/avxvnniint16-intrinsics.ll:3
+; RUN: llc < %s -verify-machineinstrs -mtriple=x86_64-unknown-unknown 
--show-mc-encoding -mattr=+avx2,+avxvnniint16 | FileCheck %s 
--check-prefixes=X64
+; RUN: llc < %s -verify-machineinstrs -mtriple=i686-unknown-unknown 
--show-mc-encoding -mattr=+avx2,+avxvnniint16 | FileCheck %s 
--check-prefixes=X86
+

pengfei wrote:
> `X86,CHECK`
I thought the common prefix had to be first? But I might be wrong


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155145

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

[PATCH] D154596: [RISCV] Fix required features checking with empty string

[Jim Lin via Phabricator via cfe-commits]

Jim added a comment.

In D154596#4499647 , @wangpc wrote:

> Can you give an example of intrinsic that doesn't require any extra extension 
> enabled?

Like read/write csr intrinsics that we add for convenient usage doesn't need 
any extra extension enabled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154596

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

[clang] 3a9683f - Fix comparison of constrained deduced return types in explicit

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2023-07-13T19:59:19-07:00
New Revision: 3a9683fce362ecfd7c4d76d4bf1198b59193e361

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

LOG: Fix comparison of constrained deduced return types in explicit
instantiations.

Fixes #62272.

Added: 


Modified: 
clang/lib/AST/Type.cpp
clang/lib/Sema/SemaTemplateDeduction.cpp
clang/test/CXX/dcl/dcl.spec/dcl.type/dcl.spec.auto/p6.cpp
clang/test/SemaObjCXX/arc-nsconsumed-errors.mm
clang/test/SemaTemplate/concepts-out-of-line-def.cpp

Removed: 




diff  --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index a1b17577fba717..99c859034423bb 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -4712,14 +4712,10 @@ AutoType::AutoType(QualType DeducedAsType, 
AutoTypeKeyword Keyword,
 auto *ArgBuffer =
 const_cast(getTypeConstraintArguments().data());
 for (const TemplateArgument  : TypeConstraintArgs) {
-  // If we have a deduced type, our constraints never affect semantic
-  // dependence. Prior to deduction, however, our canonical type depends
-  // on the template arguments, so we are a dependent type if any of them
-  // is dependent.
-  TypeDependence ArgDependence = toTypeDependence(Arg.getDependence());
-  if (!DeducedAsType.isNull())
-ArgDependence = toSyntacticDependence(ArgDependence);
-  addDependence(ArgDependence);
+  // We only syntactically depend on the constraint arguments. They don't
+  // affect the deduced type, only its validity.
+  addDependence(
+  toSyntacticDependence(toTypeDependence(Arg.getDependence(;
 
   new (ArgBuffer++) TemplateArgument(Arg);
 }

diff  --git a/clang/lib/Sema/SemaTemplateDeduction.cpp 
b/clang/lib/Sema/SemaTemplateDeduction.cpp
index 425017aa213580..31ea7be2975e49 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -1627,9 +1627,11 @@ static Sema::TemplateDeductionResult 
DeduceTemplateArgumentsByTypeMatch(
   llvm_unreachable("Type nodes handled above");
 
 case Type::Auto:
-  // FIXME: It's not clear whether we should deduce the template arguments
-  // of a constrained deduced type. For now we treat them as a non-deduced
-  // context.
+  // C++23 [temp.deduct.funcaddr]/3:
+  //   A placeholder type in the return type of a function template is a
+  //   non-deduced context.
+  // There's no corresponding wording for [temp.deduct.decl], but we treat
+  // it the same to match other compilers.
   if (P->isDependentType())
 return Sema::TDK_Success;
   [[fallthrough]];
@@ -4369,11 +4371,9 @@ Sema::TemplateDeductionResult 
Sema::DeduceTemplateArguments(
   Deduced.resize(TemplateParams->size());
 
   // If the function has a deduced return type, substitute it for a dependent
-  // type so that we treat it as a non-deduced context in what follows. If we
-  // are looking up by signature, the signature type should also have a deduced
-  // return type, which we instead expect to exactly match.
+  // type so that we treat it as a non-deduced context in what follows.
   bool HasDeducedReturnType = false;
-  if (getLangOpts().CPlusPlus14 && IsAddressOfFunction &&
+  if (getLangOpts().CPlusPlus14 &&
   Function->getReturnType()->getContainedAutoType()) {
 FunctionType = SubstAutoTypeDependent(FunctionType);
 HasDeducedReturnType = true;
@@ -4401,7 +4401,7 @@ Sema::TemplateDeductionResult 
Sema::DeduceTemplateArguments(
 
   // If the function has a deduced return type, deduce it now, so we can check
   // that the deduced function type matches the requested type.
-  if (HasDeducedReturnType &&
+  if (HasDeducedReturnType && IsAddressOfFunction &&
   Specialization->getReturnType()->isUndeducedType() &&
   DeduceReturnType(Specialization, Info.getLocation(), false))
 return TDK_MiscellaneousDeductionFailure;
@@ -4426,23 +4426,31 @@ Sema::TemplateDeductionResult 
Sema::DeduceTemplateArguments(
   // noreturn can't be dependent, so we don't actually need this for them
   // right now.)
   QualType SpecializationType = Specialization->getType();
-  if (!IsAddressOfFunction)
+  if (!IsAddressOfFunction) {
 ArgFunctionType = adjustCCAndNoReturn(ArgFunctionType, SpecializationType,
   /*AdjustExceptionSpec*/true);
 
+// Revert placeholder types in the return type back to undeduced types so
+// that the comparison below compares the declared return types.
+if (HasDeducedReturnType) {
+  SpecializationType = SubstAutoType(SpecializationType, QualType());
+  ArgFunctionType = SubstAutoType(ArgFunctionType, QualType());
+}
+  }
+
   // If the requested function type does 

[PATCH] D154596: [RISCV] Fix required features checking with empty string

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

This seems reasonable to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154596

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

[PATCH] D154596: [RISCV] Fix required features checking with empty string

[Wang Pengcheng via Phabricator via cfe-commits]

wangpc added a comment.

Can you give an example of intrinsic that doesn't require any extra extension 
enabled?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154596

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

[PATCH] D155146: Add SHA512 instructions.

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added inline comments.



Comment at: llvm/lib/Target/X86/X86.td:243
+  "Support SHA512 instructions",
+  [FeatureAVX]>;
 // Processor supports CET SHSTK - Control-Flow Enforcement Technology

craig.topper wrote:
> AVX2 like other integer features?
ISE says it only requires AVX


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155146

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

[PATCH] D146777: [clang] Preliminary fat-lto-object support

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Thanks for the update. Can you add a comment for the `-funified-lto` 
combination? It's unclear what it does...

`clang -flto=thin -ffat-lto-objects -funified-lto -fuse-ld=lld foo.c`

I've left some comments about missing test coverage.




Comment at: clang/lib/Driver/Driver.cpp:4733
+  types::ID Output;
+  if (Args.hasArg(options::OPT_S))
+Output = types::TY_LTO_IR;

This part is not tested. 



Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:621
+  } else {
+// For LLD we need to enable fat object support
+if (Args.hasArg(options::OPT_ffat_lto_objects))

Tell LLD to find and use .llvm.lto section in regular relocatable object files.



Comment at: clang/test/CodeGen/embed-fat-lto-objects.c:1
+// RUN: %clang -cc1 -triple x86_64-unknown-linux-gnu -flto=full 
-ffat-lto-objects -fsplit-lto-unit -emit-llvm < %s  | FileCheck %s 
--check-prefixes=FULL,SPLIT
+// RUN: %clang -cc1 -triple x86_64-unknown-linux-gnu -flto=full 
-ffat-lto-objects -emit-llvm < %s  | FileCheck %s --check-prefixes=FULL,SPLIT

`embed-` in the filename seems unneeded? Perhaps just `fat-lto-objects.c`



Comment at: clang/test/CodeGen/embed-fat-lto-objects.c:3
+// RUN: %clang -cc1 -triple x86_64-unknown-linux-gnu -flto=full 
-ffat-lto-objects -emit-llvm < %s  | FileCheck %s --check-prefixes=FULL,SPLIT
+
+// RUN: %clang -cc1 -triple x86_64-unknown-linux-gnu -flto=thin 
-fsplit-lto-unit -ffat-lto-objects -emit-llvm < %s  | FileCheck %s 
--check-prefixes=THIN,SPLIT

We need a `-emit-obj` test with `// REQUIRES: x86-registered-target`.
Use llvm-readelf to check that .llvm.lto section is present.

We also need a `-S` test, otherwise the behavior of driver `clang -S 
-ffat-lto-objects` is untested.





Comment at: clang/test/Driver/fat-lto-objects.c:10
+// CHECK-CC-NOLTO-NOT: -ffat-lto-objects
+// CHECK-CC-NOLTO-NOT: warning: argument unused during compilation: 
'-ffat-lto-objects'
+

This NOT pattern has no effect as warnings are usually emitted before -cc1.

You can use `--implicit-check-not=warning:`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146777

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

[PATCH] D140727: [XRay] Add initial support for loongarch64

[Limin Zhang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGef33d6cbfc2d: [XRay] Add initial support for loongarch64 
(authored by SixWeining, committed by Ami-zhang).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140727

Files:
  clang/lib/Driver/XRayArgs.cpp
  compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake
  compiler-rt/lib/xray/CMakeLists.txt
  compiler-rt/lib/xray/xray_interface.cpp
  compiler-rt/lib/xray/xray_loongarch64.cpp
  compiler-rt/lib/xray/xray_trampoline_loongarch64.S
  compiler-rt/lib/xray/xray_tsc.h
  compiler-rt/test/xray/TestCases/Posix/c-test.cpp
  compiler-rt/test/xray/TestCases/Posix/fdr-thread-order.cpp
  llvm/lib/CodeGen/XRayInstrumentation.cpp
  llvm/lib/Target/LoongArch/LoongArchAsmPrinter.cpp
  llvm/lib/Target/LoongArch/LoongArchAsmPrinter.h
  llvm/lib/Target/LoongArch/LoongArchSubtarget.h
  llvm/lib/XRay/InstrumentationMap.cpp
  llvm/test/CodeGen/LoongArch/xray-attribute-instrumentation.ll

Index: llvm/test/CodeGen/LoongArch/xray-attribute-instrumentation.ll
===
--- /dev/null
+++ llvm/test/CodeGen/LoongArch/xray-attribute-instrumentation.ll
@@ -0,0 +1,56 @@
+; RUN: llc --mtriple=loongarch64 %s -o - | FileCheck %s
+; RUN: llc --mtriple=loongarch64 -filetype=obj %s -o %t
+; RUN: llvm-readobj -r %t | FileCheck %s --check-prefix=RELOC
+
+define i32 @foo() nounwind noinline uwtable "function-instrument"="xray-always" {
+; CHECK-LABEL: foo:
+; CHECK-LABEL: .Lfunc_begin0:
+; CHECK:   .p2align 2
+; CHECK-LABEL: .Lxray_sled_begin0:
+; CHECK-NEXT:  b .Lxray_sled_end0
+; CHECK-COUNT-11:  nop
+; CHECK-LABEL: .Lxray_sled_end0:
+  ret i32 0
+; CHECK-LABEL: .Lxray_sled_begin1:
+; CHECK-NEXT:  b .Lxray_sled_end1
+; CHECK-COUNT-11:  nop
+; CHECK-NEXT: .Lxray_sled_end1:
+; CHECK-NEXT:  ret
+; CHECK-NEXT: .Lfunc_end0:
+}
+
+; CHECK-LABEL: .section xray_instr_map
+; CHECK-NEXT: .Lxray_sleds_start0:
+; CHECK-NEXT: [[TMP:.Ltmp[0-9]+]]:
+; CHECK-NEXT: .dword .Lxray_sled_begin0-[[TMP]]
+; CHECK-NEXT: .dword .Lfunc_begin0-([[TMP]]+8)
+; CHECK-NEXT: .byte 0x00
+; CHECK-NEXT: .byte 0x01
+; CHECK-NEXT: .byte 0x02
+; CHECK-NEXT: .space 13
+; CHECK-NEXT: [[TMP:.Ltmp[0-9]+]]:
+; CHECK-NEXT: .dword .Lxray_sled_begin1-[[TMP]]
+; CHECK-NEXT: .dword .Lfunc_begin0-([[TMP]]+8)
+; CHECK-NEXT: .byte 0x01
+; CHECK-NEXT: .byte 0x01
+; CHECK-NEXT: .byte 0x02
+; CHECK-NEXT: .space 13
+; CHECK-NEXT: .Lxray_sleds_end0:
+
+; CHECK-LABEL:  .section xray_fn_idx
+; CHECK:  [[IDX:.Lxray_fn_idx[0-9]+]]:
+; CHECK:  .dword .Lxray_sleds_start0-[[IDX]]
+; CHECK-NEXT: .dword 2
+
+; RELOC:  Section ([[#]]) .relaxray_instr_map {
+; RELOC-NEXT:   0x0 R_LARCH_64_PCREL .text 0x0
+; RELOC-NEXT:   0x8 R_LARCH_64_PCREL .text 0x0
+; RELOC-NEXT:   0x20 R_LARCH_64_PCREL .text 0x34
+; RELOC-NEXT:   0x28 R_LARCH_64_PCREL .text 0x0
+; RELOC-NEXT: }
+; RELOC-NEXT: Section ([[#]]) .relaxray_fn_idx {
+; RELOC-NEXT:   0x0 R_LARCH_64_PCREL xray_instr_map 0x0
+; RELOC-NEXT: }
+; RELOC-NEXT: Section ([[#]]) .rela.eh_frame {
+; RELOC-NEXT:   0x1C R_LARCH_32_PCREL .text 0x0
+; RELOC-NEXT: }
Index: llvm/lib/XRay/InstrumentationMap.cpp
===
--- llvm/lib/XRay/InstrumentationMap.cpp
+++ llvm/lib/XRay/InstrumentationMap.cpp
@@ -60,6 +60,7 @@
   // Find the section named "xray_instr_map".
   if ((!ObjFile.getBinary()->isELF() && !ObjFile.getBinary()->isMachO()) ||
   !(ObjFile.getBinary()->getArch() == Triple::x86_64 ||
+ObjFile.getBinary()->getArch() == Triple::loongarch64 ||
 ObjFile.getBinary()->getArch() == Triple::ppc64le ||
 ObjFile.getBinary()->getArch() == Triple::arm ||
 ObjFile.getBinary()->getArch() == Triple::aarch64))
Index: llvm/lib/Target/LoongArch/LoongArchSubtarget.h
===
--- llvm/lib/Target/LoongArch/LoongArchSubtarget.h
+++ llvm/lib/Target/LoongArch/LoongArchSubtarget.h
@@ -96,6 +96,7 @@
   MVT getGRLenVT() const { return GRLenVT; }
   unsigned getGRLen() const { return GRLen; }
   LoongArchABI::ABI getTargetABI() const { return TargetABI; }
+  bool isXRaySupported() const override { return is64Bit(); }
 };
 } // end namespace llvm
 
Index: llvm/lib/Target/LoongArch/LoongArchAsmPrinter.h
===
--- llvm/lib/Target/LoongArch/LoongArchAsmPrinter.h
+++ llvm/lib/Target/LoongArch/LoongArchAsmPrinter.h
@@ -42,6 +42,9 @@
  const char *ExtraCode, raw_ostream ) override;
 
   void LowerPATCHABLE_FUNCTION_ENTER(const MachineInstr );
+  void LowerPATCHABLE_FUNCTION_EXIT(const MachineInstr );
+  void LowerPATCHABLE_TAIL_CALL(const MachineInstr );
+  void emitSled(const MachineInstr , SledKind Kind);
 
   // tblgen'erated function.
   bool emitPseudoExpansionLowering(MCStreamer ,
Index: 

[PATCH] D155145: Add AVX-VNNI-INT16 instructions.

[Freddy, Ye via Phabricator via cfe-commits]

FreddyYe added inline comments.



Comment at: llvm/test/CodeGen/X86/avxvnniint16-intrinsics.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -verify-machineinstrs -mtriple=x86_64-unknown-unknown 
--show-mc-encoding -mattr=+avx2,+avxvnniint16 | FileCheck %s 
--check-prefixes=X64
+; RUN: llc < %s -verify-machineinstrs -mtriple=i686-unknown-unknown 
--show-mc-encoding -mattr=+avx2,+avxvnniint16 | FileCheck %s 
--check-prefixes=X86

pengfei wrote:
> `X64,CHECK`
This couldn't help merging the CHECKs here. Do we need it?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155145

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

[PATCH] D154596: [RISCV] Fix required features checking with empty string

[Jim Lin via Phabricator via cfe-commits]

Jim added a comment.

Kindly ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154596

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

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86-64 datalayout

[Trevor Gross via Phabricator via cfe-commits]

tmgross added a comment.

In D86310#4499095 , @rnk wrote:

> I still think we're overthinking this, and letting our ABI compat concerns 
> block us from making progress. Maybe we could do something as simple as 
> erroring out from the auto-upgrader when the module being upgraded has a 
> struct whose layout would change as a result of the increased i128 alignment, 
> while remaining bitcode compatible in the vast majority of cases.

This seems like a reasonable path forward, avoiding any concerns about IR 
mismatches while alerting users to the change. I would have to imagine there 
aren't all that many users that (1) don't use clang or another frontend that 
has to deal with this somehow, (2) use these types, (3) completely rely on 
autoupgrade.

Any i128 use, not just structs, _could_ be checked to catch mismatches like 
#50198 or the below example (more info on that in the github link I sent 
above), but this would affect clang users as well.

  void i128_val_in_0_perturbed_small(
uint8_t arg0, 
__int128_t arg1,
__int128_t arg2, 
__int128_t arg3,
__int128_t arg4, 
float arg5
  );


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

https://reviews.llvm.org/D86310

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

[PATCH] D154822: [clang] Support '-fgpu-default-stream=per-thread' for NVIDIA CUDA

[Artem Belevich via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf05b58a9468c: [clang] Support 
-fgpu-default-stream=per-thread for NVIDIA CUDA (authored by 
boxu-zhang, committed by tra).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154822

Files:
  clang/lib/CodeGen/CGCUDANV.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/CodeGenCUDA/Inputs/cuda.h
  clang/test/CodeGenCUDA/kernel-call.cu


Index: clang/test/CodeGenCUDA/kernel-call.cu
===
--- clang/test/CodeGenCUDA/kernel-call.cu
+++ clang/test/CodeGenCUDA/kernel-call.cu
@@ -2,6 +2,9 @@
 // RUN: | FileCheck %s --check-prefixes=CUDA-OLD,CHECK
 // RUN: %clang_cc1 -target-sdk-version=9.2  -emit-llvm %s -o - \
 // RUN: | FileCheck %s --check-prefixes=CUDA-NEW,CHECK
+// RUN: %clang_cc1 -target-sdk-version=9.2  -emit-llvm %s -o - \
+// RUN:   -fgpu-default-stream=per-thread -DCUDA_API_PER_THREAD_DEFAULT_STREAM 
\
+// RUN: | FileCheck %s --check-prefixes=CUDA-PTH,CHECK
 // RUN: %clang_cc1 -x hip -emit-llvm %s -o - \
 // RUN: | FileCheck %s --check-prefixes=HIP-OLD,CHECK
 // RUN: %clang_cc1 -fhip-new-launch-api -x hip -emit-llvm %s -o - \
@@ -25,6 +28,7 @@
 // CUDA-OLD: call{{.*}}cudaLaunch
 // CUDA-NEW: call{{.*}}__cudaPopCallConfiguration
 // CUDA-NEW: call{{.*}}cudaLaunchKernel
+// CUDA-PTH: call{{.*}}cudaLaunchKernel_ptsz
 __global__ void g1(int x) {}
 
 // CHECK-LABEL: define{{.*}}main
Index: clang/test/CodeGenCUDA/Inputs/cuda.h
===
--- clang/test/CodeGenCUDA/Inputs/cuda.h
+++ clang/test/CodeGenCUDA/Inputs/cuda.h
@@ -58,6 +58,10 @@
 extern "C" cudaError_t cudaLaunchKernel(const void *func, dim3 gridDim,
 dim3 blockDim, void **args,
 size_t sharedMem, cudaStream_t stream);
+extern "C" cudaError_t cudaLaunchKernel_ptsz(const void *func, dim3 gridDim,
+dim3 blockDim, void **args,
+size_t sharedMem, cudaStream_t stream);
+
 #endif
 
 extern "C" __device__ int printf(const char*, ...);
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -574,6 +574,9 @@
   Builder.defineMacro("__CLANG_RDC__");
 if (!LangOpts.HIP)
   Builder.defineMacro("__CUDA__");
+if (LangOpts.GPUDefaultStream ==
+LangOptions::GPUDefaultStreamKind::PerThread)
+  Builder.defineMacro("CUDA_API_PER_THREAD_DEFAULT_STREAM");
   }
   if (LangOpts.HIP) {
 Builder.defineMacro("__HIP__");
Index: clang/lib/CodeGen/CGCUDANV.cpp
===
--- clang/lib/CodeGen/CGCUDANV.cpp
+++ clang/lib/CodeGen/CGCUDANV.cpp
@@ -358,9 +358,13 @@
   TranslationUnitDecl *TUDecl = CGM.getContext().getTranslationUnitDecl();
   DeclContext *DC = TranslationUnitDecl::castToDeclContext(TUDecl);
   std::string KernelLaunchAPI = "LaunchKernel";
-  if (CGF.getLangOpts().HIP && CGF.getLangOpts().GPUDefaultStream ==
-   
LangOptions::GPUDefaultStreamKind::PerThread)
-KernelLaunchAPI = KernelLaunchAPI + "_spt";
+  if (CGF.getLangOpts().GPUDefaultStream ==
+  LangOptions::GPUDefaultStreamKind::PerThread) {
+if (CGF.getLangOpts().HIP)
+  KernelLaunchAPI = KernelLaunchAPI + "_spt";
+else if (CGF.getLangOpts().CUDA)
+  KernelLaunchAPI = KernelLaunchAPI + "_ptsz";
+  }
   auto LaunchKernelName = addPrefixToName(KernelLaunchAPI);
   IdentifierInfo  =
   CGM.getContext().Idents.get(LaunchKernelName);


Index: clang/test/CodeGenCUDA/kernel-call.cu
===
--- clang/test/CodeGenCUDA/kernel-call.cu
+++ clang/test/CodeGenCUDA/kernel-call.cu
@@ -2,6 +2,9 @@
 // RUN: | FileCheck %s --check-prefixes=CUDA-OLD,CHECK
 // RUN: %clang_cc1 -target-sdk-version=9.2  -emit-llvm %s -o - \
 // RUN: | FileCheck %s --check-prefixes=CUDA-NEW,CHECK
+// RUN: %clang_cc1 -target-sdk-version=9.2  -emit-llvm %s -o - \
+// RUN:   -fgpu-default-stream=per-thread -DCUDA_API_PER_THREAD_DEFAULT_STREAM \
+// RUN: | FileCheck %s --check-prefixes=CUDA-PTH,CHECK
 // RUN: %clang_cc1 -x hip -emit-llvm %s -o - \
 // RUN: | FileCheck %s --check-prefixes=HIP-OLD,CHECK
 // RUN: %clang_cc1 -fhip-new-launch-api -x hip -emit-llvm %s -o - \
@@ -25,6 +28,7 @@
 // CUDA-OLD: call{{.*}}cudaLaunch
 // CUDA-NEW: call{{.*}}__cudaPopCallConfiguration
 // CUDA-NEW: call{{.*}}cudaLaunchKernel
+// CUDA-PTH: call{{.*}}cudaLaunchKernel_ptsz
 __global__ void g1(int x) {}
 
 // CHECK-LABEL: define{{.*}}main
Index: clang/test/CodeGenCUDA/Inputs/cuda.h

[clang] f05b58a - [clang] Support '-fgpu-default-stream=per-thread' for NVIDIA CUDA

[Artem Belevich via cfe-commits]

Author: boxu.zhang
Date: 2023-07-13T16:54:57-07:00
New Revision: f05b58a9468cc2990678e06bc51df56b30344807

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

LOG: [clang] Support '-fgpu-default-stream=per-thread' for NVIDIA CUDA

I'm using clang to compile CUDA code. And just found that clang doesn't support 
the per-thread stream option for NV CUDA. I don't know if there is another 
solution.

Reviewed By: tra

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

Added: 


Modified: 
clang/lib/CodeGen/CGCUDANV.cpp
clang/lib/Frontend/InitPreprocessor.cpp
clang/test/CodeGenCUDA/Inputs/cuda.h
clang/test/CodeGenCUDA/kernel-call.cu

Removed: 




diff  --git a/clang/lib/CodeGen/CGCUDANV.cpp b/clang/lib/CodeGen/CGCUDANV.cpp
index e78fe175855e75..08769c98dc298a 100644
--- a/clang/lib/CodeGen/CGCUDANV.cpp
+++ b/clang/lib/CodeGen/CGCUDANV.cpp
@@ -358,9 +358,13 @@ void 
CGNVCUDARuntime::emitDeviceStubBodyNew(CodeGenFunction ,
   TranslationUnitDecl *TUDecl = CGM.getContext().getTranslationUnitDecl();
   DeclContext *DC = TranslationUnitDecl::castToDeclContext(TUDecl);
   std::string KernelLaunchAPI = "LaunchKernel";
-  if (CGF.getLangOpts().HIP && CGF.getLangOpts().GPUDefaultStream ==
-   
LangOptions::GPUDefaultStreamKind::PerThread)
-KernelLaunchAPI = KernelLaunchAPI + "_spt";
+  if (CGF.getLangOpts().GPUDefaultStream ==
+  LangOptions::GPUDefaultStreamKind::PerThread) {
+if (CGF.getLangOpts().HIP)
+  KernelLaunchAPI = KernelLaunchAPI + "_spt";
+else if (CGF.getLangOpts().CUDA)
+  KernelLaunchAPI = KernelLaunchAPI + "_ptsz";
+  }
   auto LaunchKernelName = addPrefixToName(KernelLaunchAPI);
   IdentifierInfo  =
   CGM.getContext().Idents.get(LaunchKernelName);

diff  --git a/clang/lib/Frontend/InitPreprocessor.cpp 
b/clang/lib/Frontend/InitPreprocessor.cpp
index 9a83bec3166001..16dd0c01bcb443 100644
--- a/clang/lib/Frontend/InitPreprocessor.cpp
+++ b/clang/lib/Frontend/InitPreprocessor.cpp
@@ -574,6 +574,9 @@ static void InitializeStandardPredefinedMacros(const 
TargetInfo ,
   Builder.defineMacro("__CLANG_RDC__");
 if (!LangOpts.HIP)
   Builder.defineMacro("__CUDA__");
+if (LangOpts.GPUDefaultStream ==
+LangOptions::GPUDefaultStreamKind::PerThread)
+  Builder.defineMacro("CUDA_API_PER_THREAD_DEFAULT_STREAM");
   }
   if (LangOpts.HIP) {
 Builder.defineMacro("__HIP__");

diff  --git a/clang/test/CodeGenCUDA/Inputs/cuda.h 
b/clang/test/CodeGenCUDA/Inputs/cuda.h
index 25f64ccefe9375..06399659c0b53e 100644
--- a/clang/test/CodeGenCUDA/Inputs/cuda.h
+++ b/clang/test/CodeGenCUDA/Inputs/cuda.h
@@ -58,6 +58,10 @@ extern "C" int __cudaPushCallConfiguration(dim3 gridSize, 
dim3 blockSize,
 extern "C" cudaError_t cudaLaunchKernel(const void *func, dim3 gridDim,
 dim3 blockDim, void **args,
 size_t sharedMem, cudaStream_t stream);
+extern "C" cudaError_t cudaLaunchKernel_ptsz(const void *func, dim3 gridDim,
+dim3 blockDim, void **args,
+size_t sharedMem, cudaStream_t stream);
+
 #endif
 
 extern "C" __device__ int printf(const char*, ...);

diff  --git a/clang/test/CodeGenCUDA/kernel-call.cu 
b/clang/test/CodeGenCUDA/kernel-call.cu
index 40407f1c29a38c..687c55a78e0047 100644
--- a/clang/test/CodeGenCUDA/kernel-call.cu
+++ b/clang/test/CodeGenCUDA/kernel-call.cu
@@ -2,6 +2,9 @@
 // RUN: | FileCheck %s --check-prefixes=CUDA-OLD,CHECK
 // RUN: %clang_cc1 -target-sdk-version=9.2  -emit-llvm %s -o - \
 // RUN: | FileCheck %s --check-prefixes=CUDA-NEW,CHECK
+// RUN: %clang_cc1 -target-sdk-version=9.2  -emit-llvm %s -o - \
+// RUN:   -fgpu-default-stream=per-thread -DCUDA_API_PER_THREAD_DEFAULT_STREAM 
\
+// RUN: | FileCheck %s --check-prefixes=CUDA-PTH,CHECK
 // RUN: %clang_cc1 -x hip -emit-llvm %s -o - \
 // RUN: | FileCheck %s --check-prefixes=HIP-OLD,CHECK
 // RUN: %clang_cc1 -fhip-new-launch-api -x hip -emit-llvm %s -o - \
@@ -25,6 +28,7 @@
 // CUDA-OLD: call{{.*}}cudaLaunch
 // CUDA-NEW: call{{.*}}__cudaPopCallConfiguration
 // CUDA-NEW: call{{.*}}cudaLaunchKernel
+// CUDA-PTH: call{{.*}}cudaLaunchKernel_ptsz
 __global__ void g1(int x) {}
 
 // CHECK-LABEL: define{{.*}}main



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

[PATCH] D154696: [Clang] Diagnose jumps into statement expressions

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

I suspect this is the same issue but our CI pointed out another instance of 
this error in some x86 code 
(https://storage.tuxsuite.com/public/clangbuiltlinux/continuous-integration2/builds/2SW8BY7moEweMJC6DJpzidlGYt8/build.log),
 which does not appear to use local labels with the same name, but rather 
unique label names. `cvise` spits out:

  void emulator_cmpxchg_emulated() {
int __ret = ({
  asm goto(" .pushsection \"__ex_table\",\"a\"\n"
   " .popsection\n"
   :
   :
   :
   : efaultu16);
  __ret;
});
  efaultu16:
({
  asm goto(" .pushsection \"__ex_table\",\"a\"\n"
   " .popsection\n"
   :
   :
   :
   : efaultu32);
efaultu32:;
});
  }



  $ clang -fsyntax-only x86.i
  x86.i:3:5: error: cannot jump from this asm goto statement to one of its 
possible targets
  3 | asm goto(" .pushsection \"__ex_table\",\"a\"\n"
| ^
  x86.i:19:3: note: possible target of asm goto statement
 19 |   efaultu32:;
|   ^
  x86.i:12:3: note: jump enters a statement expression
 12 |   ({
|   ^
  1 error generated.

which certainly seems bogus to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154696

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

[PATCH] D155241: [Fuchsia] Skip building debuginfod if httplib_ROOT is not specified

[Haowei Wu via Phabricator via cfe-commits]

haowei created this revision.
haowei added reviewers: phosek, mysterymath.
Herald added a subscriber: abrachet.
Herald added a project: All.
haowei requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch skipds building debuginfod if httplib_ROOT is not present in the 
configuration when building Fuchsia Clang toolchain.

I used list(REMOVE_ITEM) instead of list(APPEND) so user can still use 
-DLLVM_TOOLCHAIN_TOOLS to override the tools to build.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155241

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -331,6 +331,10 @@
   scan-build-py
   CACHE STRING "")
 
+if (NOT httplib_ROOT)
+  list(REMOVE_ITEM LLVM_TOOLCHAIN_TOOLS llvm-debuginfod llvm-debuginfod-find)
+endif()
+
 set(LLVM_Toolchain_DISTRIBUTION_COMPONENTS
   bolt
   clang


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -331,6 +331,10 @@
   scan-build-py
   CACHE STRING "")
 
+if (NOT httplib_ROOT)
+  list(REMOVE_ITEM LLVM_TOOLCHAIN_TOOLS llvm-debuginfod llvm-debuginfod-find)
+endif()
+
 set(LLVM_Toolchain_DISTRIBUTION_COMPONENTS
   bolt
   clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155239: [clang-format] Add SpacesInParens with SpacesInParensOptions

[Gedare Bloom via Phabricator via cfe-commits]

gedare added a comment.

After doing this, I'm not too sure about the value of keeping the higher-level 
`CStyleCasts`, `ConditionalStatements`, and `EmptyParentheses` as options to 
`SpacesInParens`. However, the behavior of `Always` is actually "Always except 
for `CStyleCasts` and `EmptyParentheses`, which is consistent with how 
`SpacesInParentheses` currently works. I think that is a bit buggy/brittle, but 
I kept the behavior as-is, and modeled the design of this after 
`SpaceBeforeParens`.

I might prefer to make `Always` really mean every single parens, and simply have
`Never`, `Always`, and `Custom`. Inside Custom, there can be a `Other` option 
to catch anything that isn't explicitly controlled.

I could make this work, mapping the previous behavior of `SpacesInParenthesis` 
to Custom with `Options = {ConditionalStatements: true, Other: true}`. I'll sit 
on this a bit, but if I don't hear objections (to the entire point of this rev, 
or to my next step), I will redo the options like this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155239

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

[PATCH] D153701: [WIP][Clang] Implement P2718R0 "Lifetime extension in range-based for loops"

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Sema/SemaExprCXX.cpp:8901-8914
+  // [P2718R0] Lifetime extension in range-based for loops.
+  //
+  // 6.7.7 [class.temporary] p5:
+  // There are four contexts in which temporaries are destroyed at a different
+  // point than the end of the full-expression.
+  //
+  // 6.7.7 [class.temporary] p6:

yronglin wrote:
> hubert.reinterpretcast wrote:
> > yronglin wrote:
> > > hubert.reinterpretcast wrote:
> > > > yronglin wrote:
> > > > > yronglin wrote:
> > > > > > hubert.reinterpretcast wrote:
> > > > > > > yronglin wrote:
> > > > > > > > rsmith wrote:
> > > > > > > > > This isn't the right way to model the behavior here -- the 
> > > > > > > > > presence or absence of an `ExprWithCleanups` is just a 
> > > > > > > > > convenience to tell consumers of the AST whether they should 
> > > > > > > > > expect to see cleanups later or not, and doesn't carry an 
> > > > > > > > > implication of affecting the actual temporary lifetimes and 
> > > > > > > > > storage durations.
> > > > > > > > > 
> > > > > > > > > The outcome that we should be aiming to reach is that all 
> > > > > > > > > `MaterializeTemporaryExpr`s created as part of processing the 
> > > > > > > > > for-range-initializer are marked as being lifetime-extended 
> > > > > > > > > by the for-range variable. Probably the simplest way to 
> > > > > > > > > handle that would be to track the current enclosing 
> > > > > > > > > for-range-initializer variable in the 
> > > > > > > > > `ExpressionEvaluationContextRecord`, and whenever a 
> > > > > > > > > `MaterializeTemporaryExpr` is created, if there is a current 
> > > > > > > > > enclosing for-range-initializer, mark that 
> > > > > > > > > `MaterializeTemporaryExpr` as being lifetime-extended by it.
> > > > > > > > Awesome! Thanks a lot for your advice, this is very helpful! I 
> > > > > > > > want to take a longer look at it.
> > > > > > > As mentioned in D139586, `CXXDefaultArgExpr`s may need additional 
> > > > > > > handling. Similarly for `CXXDefaultInitExpr`s.
> > > > > > Thanks for your tips! I have a question that what's the correct way 
> > > > > > to extent the lifetime of `CXXBindTemporaryExpr`? Can I just 
> > > > > > `materialize` the temporary? It may replaced by 
> > > > > > `MaterializeTemporaryExpr`, and then I can mark it as being 
> > > > > > lifetime-extended by the for-range variable.
> > > > > Eg.
> > > > > ```
> > > > > void f() {
> > > > >   int v[] = {42, 17, 13};
> > > > >   Mutex m;
> > > > >   for (int x : static_cast(LockGuard(m)), v) // lock released 
> > > > > in C++ 2020
> > > > >   {
> > > > > LockGuard guard(m); // OK in C++ 2020, now deadlocks
> > > > >   }
> > > > > }
> > > > > ```
> > > > > ```
> > > > > BinaryOperator 0x135036220 'int[3]' lvalue ','
> > > > > |-CXXStaticCastExpr 0x1350361d0 'void' static_cast 
> > > > > | `-CXXFunctionalCastExpr 0x135036198 'LockGuard':'struct LockGuard' 
> > > > > functional cast to LockGuard 
> > > > > |   `-CXXBindTemporaryExpr 0x135036178 'LockGuard':'struct LockGuard' 
> > > > > (CXXTemporary 0x135036178)
> > > > > | `-CXXConstructExpr 0x135036140 'LockGuard':'struct LockGuard' 
> > > > > 'void (Mutex &)'
> > > > > |   `-DeclRefExpr 0x135035e18 'Mutex':'class Mutex' lvalue Var 
> > > > > 0x135035b40 'm' 'Mutex':'class Mutex'
> > > > > `-DeclRefExpr 0x135036200 'int[3]' lvalue Var 0x135035928 'v' 'int[3]'
> > > > > ```
> > > > If `MaterializeTemporaryExpr` represents a "temporary materialization 
> > > > conversion", then the above should already have one just under the 
> > > > `static_cast` to `void` (since the cast operand would be a 
> > > > discarded-value expression).
> > > > 
> > > > There may be unfortunate effects from materializing temporaries for 
> > > > discarded-value expressions though: Technically, temporaries are also 
> > > > created for objects having scalar type.
> > > > 
> > > > Currently, `MaterializeTemporaryExpr` is documented as being tied to 
> > > > reference binding, but that is not correct: for example, 
> > > > `MaterializeTemporaryExpr` also appears when a member access is made on 
> > > > a temporary of class type.
> > > http://eel.is/c++draft/class.temporary says:
> > > ```
> > > [Note 3: Temporary objects are materialized:
> > > ...
> > > (2.6)
> > > when a prvalue that has type other than cv void appears as a 
> > > discarded-value expression ([expr.context]).
> > > — end note]
> > > ```
> > > Seems we should materialized the discard-value expression in this case, 
> > > WDYT?
> > I think we should, but what is the codegen fallout? Would no-opt builds 
> > start writing `42` into allocated memory for `static_cast(42)`?
> Thanks for your confirm @hubert.reinterpretcast ! 
> 
> I have tried locally, the generated  IR of `void f()` is:
> ```
> define void @f()() {
> entry:
>   %v = alloca [3 x i32], align 4
>   %m = alloca %class.Mutex, align 8
>   %__range1 = alloca ptr, align 

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Changing the default improves GCC compatibility (`Enabled by default under 
-std=c++14 and above.`) and I think is the right direction.

@wangpc You can use your new username to commandeer this revision?

To other reviewers, what's still blocked now that the Apple target issue 
appears to be addressed?
Note, we could also keep Apple behavior unchanged in this patch and let folks 
more familiar with it to handle the change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112921

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

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86-64 datalayout

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

I'm not personally involved with any workflows that care about autoupgrade, so 
I'm not really invested in ensuring it's stable.  If everyone agrees the impact 
is small enough that we're willing to just break autoupgrade to the extent it's 
relevant, I'll withdraw my objection.

> As for the longer term solution to this problem, instead of permitting mixed 
> data layouts of data layout customization, IMO LLVM structs should explicitly 
> encode field offsets. LLVM would still have APIs to assist frontends with 
> producing semi-C-compatible struct layouts, in so much as we do today.

I agree with the general sentiment that we want less dependence on alignment 
specified in the datalayout.  Not sure about the exact design of that for 
structs... if IR moves in the direction people are proposing, the notion of a 
"struct" with a memory layout will likely go away altogether.  (If we remove 
struct types form global variables and GEPs, there's very little left that 
actually cares about the layout of structs in memory.)


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

https://reviews.llvm.org/D86310

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

[PATCH] D155239: [clang-format] Add SpacesInParens with SpacesInParensOptions

[Gedare Bloom via Phabricator via cfe-commits]

gedare created this revision.
Herald added projects: All, clang, clang-format.
Herald added a subscriber: cfe-commits.
Herald added reviewers: rymiel, HazardyKnusperkeks, owenpan, MyDeveloperDay.
gedare requested review of this revision.

This is a refactoring of:

- SpacesInConditionalStatement
- SpacesInCStyleCastParentheses
- SpaceInEmptyParentheses
- SpacesInParentheses

These are now options under the new Style Option: SpacesInParens. The existing
options are maintained for backward compatibility.

Within SpacesInParens, there are currently options for:

- Never
- ConditionalStatements
- CStyleCasts
- EmptyParentheses
- Always
- Custom

Always enables the same space additions as SpacesInParentheses. The currently
available options for Custom are:

- InConditionalStatements
- InCStyleCasts
- InEmptyParentheses

This refactoring does not add or remove any existing features, but it makes it
possible to more easily extend and maintain the addition of spaces within
parentheses.

This rev is related to Github Issue 55428.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155239

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/ConfigParseTest.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11038,7 +11038,7 @@
   verifyFormat("SomeType MemberFunction(const Deleted &) &;", Spaces);
 
   Spaces.SpacesInCStyleCastParentheses = false;
-  Spaces.SpacesInParentheses = true;
+  Spaces.SpacesInParens = FormatStyle::SIPO_Always;
   verifyFormat("Deleted =( const Deleted & ) & = default;", Spaces);
   verifyFormat("SomeType MemberFunction( const Deleted & ) & = delete;",
Spaces);
@@ -13674,7 +13674,7 @@
 
   FormatStyle SpaceBetweenBraces = getLLVMStyle();
   SpaceBetweenBraces.SpacesInAngles = FormatStyle::SIAS_Always;
-  SpaceBetweenBraces.SpacesInParentheses = true;
+  SpaceBetweenBraces.SpacesInParens = FormatStyle::SIPO_Always;
   SpaceBetweenBraces.SpacesInSquareBrackets = true;
   verifyFormat("vector< int > x{ 1, 2, 3, 4 };", SpaceBetweenBraces);
   verifyFormat("f( {}, { {}, {} }, MyMap[ { k, v } ] );", SpaceBetweenBraces);
@@ -16707,10 +16707,10 @@
   verifyFormat("! ! x", Spaces);
 }
 
-TEST_F(FormatTest, ConfigurableSpacesInParentheses) {
+TEST_F(FormatTest, ConfigurableSpacesInParens) {
   FormatStyle Spaces = getLLVMStyle();
 
-  Spaces.SpacesInParentheses = true;
+  Spaces.SpacesInParens = FormatStyle::SIPO_Always;
   verifyFormat("do_something( ::globalVar );", Spaces);
   verifyFormat("call( x, y, z );", Spaces);
   verifyFormat("call();", Spaces);
@@ -16738,7 +16738,7 @@
"}",
Spaces);
 
-  Spaces.SpacesInParentheses = false;
+  Spaces.SpacesInParens = FormatStyle::SIPO_Never;
   Spaces.SpacesInCStyleCastParentheses = true;
   verifyFormat("Type *A = ( Type * )P;", Spaces);
   verifyFormat("Type *A = ( vector )P;", Spaces);
@@ -16750,7 +16750,7 @@
   verifyFormat("#define x (( int )-1)", Spaces);
 
   // Run the first set of tests again with:
-  Spaces.SpacesInParentheses = false;
+  Spaces.SpacesInParens = FormatStyle::SIPO_Never;
   Spaces.SpaceInEmptyParentheses = true;
   Spaces.SpacesInCStyleCastParentheses = true;
   verifyFormat("call(x, y, z);", Spaces);
@@ -23523,10 +23523,10 @@
   verifyFormat("vector<_Atomic(uint64_t)* attr> x;", Style);
 
   Style.SpacesInCStyleCastParentheses = true;
-  Style.SpacesInParentheses = false;
+  Style.SpacesInParens = FormatStyle::SIPO_Never;
   verifyFormat("x = ( _Atomic(uint64_t) )*a;", Style);
   Style.SpacesInCStyleCastParentheses = false;
-  Style.SpacesInParentheses = true;
+  Style.SpacesInParens = FormatStyle::SIPO_Always;
   verifyFormat("x = (_Atomic( uint64_t ))*a;", Style);
   verifyFormat("x = (_Atomic( uint64_t ))", Style);
 }
Index: clang/unittests/Format/ConfigParseTest.cpp
===
--- clang/unittests/Format/ConfigParseTest.cpp
+++ clang/unittests/Format/ConfigParseTest.cpp
@@ -175,7 +175,6 @@
   CHECK_PARSE_BOOL(ReflowComments);
   CHECK_PARSE_BOOL(RemoveBracesLLVM);
   CHECK_PARSE_BOOL(RemoveSemicolon);
-  CHECK_PARSE_BOOL(SpacesInParentheses);
   CHECK_PARSE_BOOL(SpacesInSquareBrackets);
   CHECK_PARSE_BOOL(SpacesInConditionalStatement);
   CHECK_PARSE_BOOL(SpaceInEmptyBlock);
@@ -221,6 +220,9 @@
   CHECK_PARSE_NESTED_BOOL(SpaceBeforeParensOptions, AfterIfMacros);
   CHECK_PARSE_NESTED_BOOL(SpaceBeforeParensOptions, AfterOverloadedOperator);
   CHECK_PARSE_NESTED_BOOL(SpaceBeforeParensOptions, BeforeNonEmptyParentheses);
+  CHECK_PARSE_NESTED_BOOL(SpacesInParensOptions, InCStyleCasts);
+  CHECK_PARSE_NESTED_BOOL(SpacesInParensOptions, InConditionalStatements);
+  

[PATCH] D146777: [clang] Preliminary fat-lto-object support

[Paul Kirth via Phabricator via cfe-commits]

paulkirth updated this revision to Diff 540192.
paulkirth marked an inline comment as done.
paulkirth edited the summary of this revision.
paulkirth added a comment.

Update summary w/ instructions for using `-ffat-lto-objects`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146777

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/CodeGen/embed-fat-lto-objects.c
  clang/test/Driver/clang_f_opts.c
  clang/test/Driver/fat-lto-objects.c

Index: clang/test/Driver/fat-lto-objects.c
===
--- /dev/null
+++ clang/test/Driver/fat-lto-objects.c
@@ -0,0 +1,19 @@
+// RUN: %clang --target=x86_64-unknown-linux-gnu -flto -ffat-lto-objects -### %s -c 2>&1 | FileCheck %s -check-prefix=CHECK-CC
+// CHECK-CC: -cc1
+// CHECK-CC-SAME: -emit-obj
+// CHECK-CC-SAME: -ffat-lto-objects
+
+// RUN: %clang --target=x86_64-unknown-linux-gnu -ffat-lto-objects -### %s -c 2>&1 | FileCheck %s -check-prefix=CHECK-CC-NOLTO
+// CHECK-CC-NOLTO: -cc1
+// CHECK-CC-NOLTO-SAME: -emit-obj
+// CHECK-CC-NOLTO-NOT: -ffat-lto-objects
+// CHECK-CC-NOLTO-NOT: warning: argument unused during compilation: '-ffat-lto-objects'
+
+/// We need to pass an additional flag to lld when linking w/ -flto -ffat-lto-objects
+/// But it should not be there when LTO is disabled w/ -fno-lto
+// RUN: %clang --target=x86_64-unknown-linux-gnu --sysroot=%S/Inputs/basic_cross_linux_tree %s \
+// RUN:   -fuse-ld=lld -flto -ffat-lto-objects -### 2>&1 | FileCheck --check-prefix=LTO %s
+// RUN: %clang --target=x86_64-unknown-linux-gnu --sysroot=%S/Inputs/basic_cross_linux_tree %s \
+// RUN:   -fuse-ld=lld -fno-lto -ffat-lto-objects -### 2>&1 | FileCheck --check-prefix=NOLTO %s
+// LTO: "--fat-lto-objects"
+// NOLTO-NOT: "--fat-lto-objects"
Index: clang/test/Driver/clang_f_opts.c
===
--- clang/test/Driver/clang_f_opts.c
+++ clang/test/Driver/clang_f_opts.c
@@ -424,7 +424,6 @@
 // CHECK-WARNING-DAG: optimization flag '-fwhole-program' is not supported
 // CHECK-WARNING-DAG: optimization flag '-fcaller-saves' is not supported
 // CHECK-WARNING-DAG: optimization flag '-freorder-blocks' is not supported
-// CHECK-WARNING-DAG: optimization flag '-ffat-lto-objects' is not supported
 // CHECK-WARNING-DAG: optimization flag '-fmerge-constants' is not supported
 // CHECK-WARNING-DAG: optimization flag '-finline-small-functions' is not supported
 // CHECK-WARNING-DAG: optimization flag '-ftree-dce' is not supported
Index: clang/test/CodeGen/embed-fat-lto-objects.c
===
--- /dev/null
+++ clang/test/CodeGen/embed-fat-lto-objects.c
@@ -0,0 +1,17 @@
+// RUN: %clang -cc1 -triple x86_64-unknown-linux-gnu -flto=full -ffat-lto-objects -fsplit-lto-unit -emit-llvm < %s  | FileCheck %s --check-prefixes=FULL,SPLIT
+// RUN: %clang -cc1 -triple x86_64-unknown-linux-gnu -flto=full -ffat-lto-objects -emit-llvm < %s  | FileCheck %s --check-prefixes=FULL,SPLIT
+
+// RUN: %clang -cc1 -triple x86_64-unknown-linux-gnu -flto=thin -fsplit-lto-unit -ffat-lto-objects -emit-llvm < %s  | FileCheck %s --check-prefixes=THIN,SPLIT
+// RUN: %clang -cc1 -triple x86_64-unknown-linux-gnu -flto=thin -ffat-lto-objects -emit-llvm < %s  | FileCheck %s --check-prefixes=THIN,NOSPLIT
+
+/// Check that the ThinLTO metadata is only set false for full LTO.
+// FULL: ![[#]] = !{i32 1, !"ThinLTO", i32 0}
+// THIN-NOT: ![[#]] = !{i32 1, !"ThinLTO", i32 0}
+
+/// Be sure we enable split LTO unints correctly under -ffat-lto-objects.
+// SPLIT: ![[#]] = !{i32 1, !"EnableSplitLTOUnit", i32 1}
+// NOSPLIT: ![[#]] = !{i32 1, !"EnableSplitLTOUnit", i32 0}
+
+int test(void) {
+  return 0xabcd;
+}
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -617,6 +617,10 @@
 PluginName + Suffix,
 Plugin);
 CmdArgs.push_back(Args.MakeArgString(Twine(PluginPrefix) + Plugin));
+  } else {
+// For LLD we need to enable fat object support
+if (Args.hasArg(options::OPT_ffat_lto_objects))
+  CmdArgs.push_back("--fat-lto-objects");
   }
 
   const char *PluginOptPrefix = IsOSAIX ? "-bplugin_opt:" : "-plugin-opt=";
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7354,6 +7354,22 @@
   if (SplitLTOUnit)
 CmdArgs.push_back("-fsplit-lto-unit");
 
+  if (Arg *A = 

[clang] 6504d87 - [clang][modules] Deserialize included files lazily

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2023-07-13T15:00:11-07:00
New Revision: 6504d87fc0c89fc584c2128355a14a07ed385c5b

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

LOG: [clang][modules] Deserialize included files lazily

In D114095, `HeaderFileInfo::NumIncludes` was moved into `Preprocessor`. This 
still makes sense, because we want to track this on the granularity of 
submodules (D112915, D114173), but the way this information is serialized is 
not ideal. In `ASTWriter`, the set of included files gets deserialized eagerly, 
issuing lots of calls to `FileManager::getFile()` for input files the PCM 
consumer might not be interested in.

This patch makes the information part of the header file info table, taking 
advantage of its lazy deserialization which typically happens when a file is 
about to be included.

Reviewed By: benlangmuir

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

Added: 


Modified: 
clang/include/clang/Lex/Preprocessor.h
clang/include/clang/Serialization/ASTBitCodes.h
clang/include/clang/Serialization/ASTReader.h
clang/include/clang/Serialization/ASTWriter.h
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTReaderInternals.h
clang/lib/Serialization/ASTWriter.cpp

Removed: 




diff  --git a/clang/include/clang/Lex/Preprocessor.h 
b/clang/include/clang/Lex/Preprocessor.h
index 8fbc002059a86b..9efe439bc5f219 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -1486,6 +1486,7 @@ class Preprocessor {
 
   /// Return true if this header has already been included.
   bool alreadyIncluded(const FileEntry *File) const {
+HeaderInfo.getFileInfo(File);
 return IncludedFiles.count(File);
   }
 

diff  --git a/clang/include/clang/Serialization/ASTBitCodes.h 
b/clang/include/clang/Serialization/ASTBitCodes.h
index 7019bc5922ebcb..074d1002913084 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -41,7 +41,7 @@ namespace serialization {
 /// Version 4 of AST files also requires that the version control branch and
 /// revision match exactly, since there is no backward compatibility of
 /// AST files at this time.
-const unsigned VERSION_MAJOR = 25;
+const unsigned VERSION_MAJOR = 26;
 
 /// AST file minor version number supported by this version of
 /// Clang.
@@ -696,8 +696,7 @@ enum ASTRecordTypes {
   /// Record code for \#pragma float_control options.
   FLOAT_CONTROL_PRAGMA_OPTIONS = 65,
 
-  /// Record code for included files.
-  PP_INCLUDED_FILES = 66,
+  /// ID 66 used to be the list of included files.
 
   /// Record code for an unterminated \#pragma clang assume_nonnull begin
   /// recorded in a preamble.

diff  --git a/clang/include/clang/Serialization/ASTReader.h 
b/clang/include/clang/Serialization/ASTReader.h
index 85f31ed22aca9d..d56e2117a53f0d 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -1391,7 +1391,6 @@ class ASTReader
   void ParseLineTable(ModuleFile , const RecordData );
   llvm::Error ReadSourceManagerBlock(ModuleFile );
   SourceLocation getImportLocation(ModuleFile *F);
-  void readIncludedFiles(ModuleFile , StringRef Blob, Preprocessor );
   ASTReadResult ReadModuleMapFileBlock(RecordData , ModuleFile ,
const ModuleFile *ImportedBy,
unsigned ClientLoadCapabilities);

diff  --git a/clang/include/clang/Serialization/ASTWriter.h 
b/clang/include/clang/Serialization/ASTWriter.h
index 09ee1744e8945f..e328dd0cd5577e 100644
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -500,7 +500,6 @@ class ASTWriter : public ASTDeserializationListener,
   void WriteInputFiles(SourceManager , HeaderSearchOptions );
   void WriteSourceManagerBlock(SourceManager ,
const Preprocessor );
-  void writeIncludedFiles(raw_ostream , const Preprocessor );
   void WritePreprocessor(const Preprocessor , bool IsModule);
   void WriteHeaderSearch(const HeaderSearch );
   void WritePreprocessorDetail(PreprocessingRecord ,

diff  --git a/clang/lib/Serialization/ASTReader.cpp 
b/clang/lib/Serialization/ASTReader.cpp
index b989ff2a9c95c4..380d117acc4973 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -1875,6 +1875,21 @@ ASTReader::getGlobalPreprocessedEntityID(ModuleFile ,
   return LocalID + I->second;
 }
 
+const FileEntry *HeaderFileInfoTrait::getFile(const internal_key_type ) {
+  FileManager  = Reader.getFileManager();
+  if (!Key.Imported) {
+if (auto File = FileMgr.getFile(Key.Filename))
+  return *File;
+return 

[PATCH] D155131: [clang][modules] Deserialize included files lazily

[Jan Svoboda 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 rG6504d87fc0c8: [clang][modules] Deserialize included files 
lazily (authored by jansvoboda11).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155131

Files:
  clang/include/clang/Lex/Preprocessor.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/ASTReader.h
  clang/include/clang/Serialization/ASTWriter.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderInternals.h
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -866,7 +866,6 @@
   RECORD(CUDA_PRAGMA_FORCE_HOST_DEVICE_DEPTH);
   RECORD(PP_CONDITIONAL_STACK);
   RECORD(DECLS_TO_CHECK_FOR_DEFERRED_DIAGS);
-  RECORD(PP_INCLUDED_FILES);
   RECORD(PP_ASSUME_NONNULL_LOC);
 
   // SourceManager Block.
@@ -1763,6 +1762,7 @@
 
 struct data_type {
   const HeaderFileInfo 
+  bool AlreadyIncluded;
   ArrayRef KnownHeaders;
   UnresolvedModule Unresolved;
 };
@@ -1808,7 +1808,8 @@
   endian::Writer LE(Out, little);
   uint64_t Start = Out.tell(); (void)Start;
 
-  unsigned char Flags = (Data.HFI.isImport << 5)
+  unsigned char Flags = (Data.AlreadyIncluded << 6)
+  | (Data.HFI.isImport << 5)
   | (Data.HFI.isPragmaOnce << 4)
   | (Data.HFI.DirInfo << 1)
   | Data.HFI.IndexHeaderMapHeader;
@@ -1909,7 +1910,7 @@
 HeaderFileInfoTrait::key_type Key = {
 FilenameDup, *U.Size, IncludeTimestamps ? *U.ModTime : 0};
 HeaderFileInfoTrait::data_type Data = {
-Empty, {}, {M, ModuleMap::headerKindToRole(U.Kind)}};
+Empty, false, {}, {M, ModuleMap::headerKindToRole(U.Kind)}};
 // FIXME: Deal with cases where there are multiple unresolved header
 // directives in different submodules for the same header.
 Generator.insert(Key, Data, GeneratorTrait);
@@ -1952,11 +1953,13 @@
   SavedStrings.push_back(Filename.data());
 }
 
+bool Included = PP->alreadyIncluded(File);
+
 HeaderFileInfoTrait::key_type Key = {
   Filename, File->getSize(), getTimestampForOutput(File)
 };
 HeaderFileInfoTrait::data_type Data = {
-  *HFI, HS.getModuleMap().findResolvedModulesForHeader(File), {}
+  *HFI, Included, HS.getModuleMap().findResolvedModulesForHeader(File), {}
 };
 Generator.insert(Key, Data, GeneratorTrait);
 ++NumHeaderSearchEntries;
@@ -2262,29 +2265,6 @@
   return false;
 }
 
-void ASTWriter::writeIncludedFiles(raw_ostream , const Preprocessor ) {
-  using namespace llvm::support;
-
-  const Preprocessor::IncludedFilesSet  = PP.getIncludedFiles();
-
-  std::vector IncludedInputFileIDs;
-  IncludedInputFileIDs.reserve(IncludedFiles.size());
-
-  for (const FileEntry *File : IncludedFiles) {
-auto InputFileIt = InputFileIDs.find(File);
-if (InputFileIt == InputFileIDs.end())
-  continue;
-IncludedInputFileIDs.push_back(InputFileIt->second);
-  }
-
-  llvm::sort(IncludedInputFileIDs);
-
-  endian::Writer LE(Out, little);
-  LE.write(IncludedInputFileIDs.size());
-  for (uint32_t ID : IncludedInputFileIDs)
-LE.write(ID);
-}
-
 /// Writes the block containing the serialized form of the
 /// preprocessor.
 void ASTWriter::WritePreprocessor(const Preprocessor , bool IsModule) {
@@ -2533,20 +2513,6 @@
MacroOffsetsBase - ASTBlockStartOffset};
 Stream.EmitRecordWithBlob(MacroOffsetAbbrev, Record, bytes(MacroOffsets));
   }
-
-  {
-auto Abbrev = std::make_shared();
-Abbrev->Add(BitCodeAbbrevOp(PP_INCLUDED_FILES));
-Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob));
-unsigned IncludedFilesAbbrev = Stream.EmitAbbrev(std::move(Abbrev));
-
-SmallString<2048> Buffer;
-raw_svector_ostream Out(Buffer);
-writeIncludedFiles(Out, PP);
-RecordData::value_type Record[] = {PP_INCLUDED_FILES};
-Stream.EmitRecordWithBlob(IncludedFilesAbbrev, Record, Buffer.data(),
-  Buffer.size());
-  }
 }
 
 void ASTWriter::WritePreprocessorDetail(PreprocessingRecord ,
Index: clang/lib/Serialization/ASTReaderInternals.h
===
--- clang/lib/Serialization/ASTReaderInternals.h
+++ clang/lib/Serialization/ASTReaderInternals.h
@@ -276,6 +276,9 @@
   static internal_key_type ReadKey(const unsigned char *d, unsigned);
 
   data_type ReadData(internal_key_ref,const unsigned char *d, unsigned DataLen);
+
+private:
+  const FileEntry *getFile(const internal_key_type );
 };
 
 /// The on-disk hash table used for known 

[PATCH] D148654: Modify BoundsSan to improve debuggability

[Oskar Wirga via Phabricator via cfe-commits]

oskarwirga added inline comments.



Comment at: clang/lib/CodeGen/CGExpr.cpp:3594
+TrapCall->addFnAttr(A);
+  }
+  TrapCall->setDoesNotReturn();

aeubanks wrote:
> oskarwirga wrote:
> > vitalybuka wrote:
> > > wouldn't be you issues solved with
> > > TrapCall->setCannotMerge() here?
> > ubsantrap gets lowered to an instruction in MIR which then gets merged 
> > later. This was the only way I was able to create unique instruction per 
> > check. 
> isn't that a `nomerge` bug that should get fixed instead?
I don't think so because nomerge is a function attribute and the ubsantrap 
intrinsic gets lowered as an instruction. Creating instruction attributes from 
lowered function attributes seems a bit involved to me. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148654

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

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86-64 datalayout

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In D86310#4498575 , @efriedma wrote:

> https://reviews.llvm.org/D86310#2231136 has an example where IR generated by 
> clang breaks.

Right, so we'd break LTO of packed structs with i128 members.

I still think we're overthinking this, and letting our ABI compat concerns 
block us from making progress. Maybe we could do something as simple as 
erroring out from the auto-upgrader when the module being upgraded has a struct 
whose layout would change as a result of the increased i128 alignment, while 
remaining bitcode compatible in the vast majority of cases.

As for the longer term solution to this problem, instead of permitting mixed 
data layouts of data layout customization, IMO LLVM structs should explicitly 
encode field offsets. LLVM would still have APIs to assist frontends with 
producing semi-C-compatible struct layouts, in so much as we do today.


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

https://reviews.llvm.org/D86310

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

[PATCH] D148654: Modify BoundsSan to improve debuggability

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added inline comments.



Comment at: clang/lib/CodeGen/CGExpr.cpp:3594
+TrapCall->addFnAttr(A);
+  }
+  TrapCall->setDoesNotReturn();

oskarwirga wrote:
> vitalybuka wrote:
> > wouldn't be you issues solved with
> > TrapCall->setCannotMerge() here?
> ubsantrap gets lowered to an instruction in MIR which then gets merged later. 
> This was the only way I was able to create unique instruction per check. 
isn't that a `nomerge` bug that should get fixed instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148654

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

[PATCH] D155081: Specify the developer policy around links to external resources

[Chris Lattner via Phabricator via cfe-commits]

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

Thank you so much for raising this issue Aaron.

Documenting the policy in the developer policy is totally the right thing to do 
given this cross-cuts individual subprojects, but I don't think we have 
consensus on what the right approach is.  This is a pretty important issue to a 
number of LLVM contributors, and I think we should have a first-principles 
discussion about it on the forum before instituting a policy.
https://discourse.llvm.org/t/code-review-reminder-about-links-in-code-commit-messages/71847/43?u=clattner


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155081

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

[PATCH] D148654: Modify BoundsSan to improve debuggability

[Oskar Wirga via Phabricator via cfe-commits]

oskarwirga updated this revision to Diff 540181.
oskarwirga added a comment.

Refactor CodeGen code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148654

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/test/CodeGen/bounds-checking.c
  llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp

Index: llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp
===
--- llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp
+++ llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp
@@ -37,6 +37,10 @@
 static cl::opt SingleTrapBB("bounds-checking-single-trap",
   cl::desc("Use one trap block per function"));
 
+static cl::opt
+DebugTrapBB("bounds-checking-debug-trap",
+cl::desc("Use one trap block per check despite optimizations"));
+
 STATISTIC(ChecksAdded, "Bounds checks added");
 STATISTIC(ChecksSkipped, "Bounds checks skipped");
 STATISTIC(ChecksUnable, "Bounds checks unable to add");
@@ -180,24 +184,39 @@
   // will create a fresh block every time it is called.
   BasicBlock *TrapBB = nullptr;
   auto GetTrapBB = [](BuilderTy ) {
-if (TrapBB && SingleTrapBB)
-  return TrapBB;
-
-Function *Fn = IRB.GetInsertBlock()->getParent();
-// FIXME: This debug location doesn't make a lot of sense in the
-// `SingleTrapBB` case.
-auto DebugLoc = IRB.getCurrentDebugLocation();
-IRBuilder<>::InsertPointGuard Guard(IRB);
-TrapBB = BasicBlock::Create(Fn->getContext(), "trap", Fn);
-IRB.SetInsertPoint(TrapBB);
-
-auto *F = Intrinsic::getDeclaration(Fn->getParent(), Intrinsic::trap);
-CallInst *TrapCall = IRB.CreateCall(F, {});
-TrapCall->setDoesNotReturn();
-TrapCall->setDoesNotThrow();
-TrapCall->setDebugLoc(DebugLoc);
-IRB.CreateUnreachable();
-
+if (DebugTrapBB) {
+  Function *Fn = IRB.GetInsertBlock()->getParent();
+  auto DebugLoc = IRB.getCurrentDebugLocation();
+  IRBuilder<>::InsertPointGuard Guard(IRB);
+  TrapBB = BasicBlock::Create(Fn->getContext(), "trap", Fn);
+  IRB.SetInsertPoint(TrapBB);
+  auto *F =
+  Intrinsic::getDeclaration(Fn->getParent(), Intrinsic::ubsantrap);
+  CallInst *TrapCall =
+  IRB.CreateCall(F, ConstantInt::get(IRB.getInt8Ty(), Fn->size()));
+  TrapCall->setDoesNotReturn();
+  TrapCall->setDoesNotThrow();
+  TrapCall->setDebugLoc(DebugLoc);
+  IRB.CreateUnreachable();
+} else {
+  if (TrapBB && SingleTrapBB)
+return TrapBB;
+
+  Function *Fn = IRB.GetInsertBlock()->getParent();
+  // FIXME: This debug location doesn't make a lot of sense in the
+  // `SingleTrapBB` case.
+  auto DebugLoc = IRB.getCurrentDebugLocation();
+  IRBuilder<>::InsertPointGuard Guard(IRB);
+  TrapBB = BasicBlock::Create(Fn->getContext(), "trap", Fn);
+  IRB.SetInsertPoint(TrapBB);
+
+  auto *F = Intrinsic::getDeclaration(Fn->getParent(), Intrinsic::trap);
+  CallInst *TrapCall = IRB.CreateCall(F, {});
+  TrapCall->setDoesNotReturn();
+  TrapCall->setDoesNotThrow();
+  TrapCall->setDebugLoc(DebugLoc);
+  IRB.CreateUnreachable();
+}
 return TrapBB;
   };
 
Index: clang/test/CodeGen/bounds-checking.c
===
--- clang/test/CodeGen/bounds-checking.c
+++ clang/test/CodeGen/bounds-checking.c
@@ -1,5 +1,7 @@
 // RUN: %clang_cc1 -fsanitize=local-bounds -emit-llvm -triple x86_64-apple-darwin10 %s -o - | FileCheck %s
 // RUN: %clang_cc1 -fsanitize=array-bounds -O -fsanitize-trap=array-bounds -emit-llvm -triple x86_64-apple-darwin10 -DNO_DYNAMIC %s -o - | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=local-bounds -fsanitize-trap=local-bounds -O3 -mllvm -bounds-checking-debug-trap -emit-llvm -triple x86_64-apple-darwin10 %s -o - | FileCheck %s --check-prefixes=NOOPTLOCAL
+// RUN: %clang_cc1 -fsanitize=array-bounds -fsanitize-trap=array-bounds -O3 -mllvm -sanitizer-de-opt-traps -emit-llvm -triple x86_64-apple-darwin10 %s -o - | FileCheck %s --check-prefixes=NOOPTARRAY
 //
 // REQUIRES: x86-registered-target
 
@@ -66,3 +68,16 @@
   // CHECK-NOT: @llvm.ubsantrap
   return u->c[i];
 }
+
+char B[10];
+char B2[10];
+// CHECK-LABEL: @f8
+void f8(int i, int k) {
+  // NOOPTLOCAL: call void @llvm.ubsantrap(i8 3)
+  // NOOPTARRAY: call void @llvm.ubsantrap(i8 2)
+  B[i] = '\0';
+
+  // NOOPTLOCAL: call void @llvm.ubsantrap(i8 5)
+  // NOOPTARRAY: call void @llvm.ubsantrap(i8 4)
+  B2[k] = '\0';
+}
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -51,6 +51,11 @@
 using namespace clang;
 using namespace CodeGen;
 
+// Experiment to make sanitizers easier to debug
+static llvm::cl::opt ClSanitizeDebugDeoptimization(
+"sanitizer-de-opt-traps", llvm::cl::Optional,
+

[PATCH] D153881: Create diagnostic group for definition deprecation warning

[Nuri Amari via Phabricator via cfe-commits]

nuriamari added a comment.

In D153881#4497041 , @aaron.ballman 
wrote:

> LGTM! Do you need me to land this on your behalf? If so, what name and email 
> address would you like me to use for patch attribution? (If I'm landing it, I 
> can fix up the release note for you when landing.)

I do. Nuri Amari - nuri.amar...@gmail.com would be great, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153881

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

[PATCH] D155234: [SystemZ][z/OS] Forward headers to z/os system headers

[Sean via Phabricator via cfe-commits]

SeanP created this revision.
SeanP added reviewers: abhina.sreeskantharajan, zibi, fanbo-meng.
Herald added a project: All.
SeanP requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, jplehr, sstefan1.
Herald added a project: clang.

For the headers that have a z/os system header add include_next so the system 
headers are used.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155234

Files:
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Headers/builtins.h
  clang/lib/Headers/float.h
  clang/lib/Headers/inttypes.h
  clang/lib/Headers/iso646.h
  clang/lib/Headers/limits.h
  clang/lib/Headers/stdalign.h
  clang/lib/Headers/stdarg.h
  clang/lib/Headers/stdbool.h
  clang/lib/Headers/stddef.h
  clang/lib/Headers/stdint.h
  clang/lib/Headers/stdnoreturn.h
  clang/lib/Headers/varargs.h
  clang/lib/Headers/zos_wrappers/builtins.h

Index: clang/lib/Headers/zos_wrappers/builtins.h
===
--- /dev/null
+++ clang/lib/Headers/zos_wrappers/builtins.h
@@ -0,0 +1,19 @@
+/*=== builtins.h - z/Architecture Builtin Functions ===
+ *
+ * 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
+ *
+ *===---===
+ */
+
+#ifndef __ZOS_WRAPPERS_BUILTINS_H
+#define __ZOS_WRAPPERS_BUILTINS_H
+  #if defined(__MVS__)
+#include_next 
+#if defined(__VEC__)
+  #include 
+#endif
+  #endif /* defined(__MVS__) */
+#endif /* __ZOS_WRAPPERS_BUILTINS_H */
+
Index: clang/lib/Headers/varargs.h
===
--- clang/lib/Headers/varargs.h
+++ clang/lib/Headers/varargs.h
@@ -8,5 +8,9 @@
 */
 #ifndef __VARARGS_H
 #define __VARARGS_H
+#if defined(__MVS__) && __has_include_next()
+  #include_next 
+#else
   #error "Please use  instead of "
+#endif /* __MVS__ */
 #endif
Index: clang/lib/Headers/stdnoreturn.h
===
--- clang/lib/Headers/stdnoreturn.h
+++ clang/lib/Headers/stdnoreturn.h
@@ -10,9 +10,15 @@
 #ifndef __STDNORETURN_H
 #define __STDNORETURN_H
 
+#if defined(__MVS__) && __has_include_next()
+#include_next 
+#else
+
 #define noreturn _Noreturn
 #define __noreturn_is_defined 1
 
+#endif /* __MVS__ */
+
 #if (defined(__STDC_VERSION__) && __STDC_VERSION__ > 201710L) &&   \
 !defined(_CLANG_DISABLE_CRT_DEPRECATION_WARNINGS)
 /* The noreturn macro is deprecated in C2x. We do not mark it as such because
Index: clang/lib/Headers/stdint.h
===
--- clang/lib/Headers/stdint.h
+++ clang/lib/Headers/stdint.h
@@ -14,6 +14,10 @@
 #define __CLANG_STDINT_H
 #endif
 
+#if defined(__MVS__) && __has_include_next()
+#include_next 
+#else
+
 /* If we're hosted, fall back to the system's stdint.h, which might have
  * additional definitions.
  */
@@ -976,4 +980,5 @@
 #endif
 
 #endif /* __STDC_HOSTED__ */
+#endif /* __MVS__ */
 #endif /* __CLANG_STDINT_H */
Index: clang/lib/Headers/stddef.h
===
--- clang/lib/Headers/stddef.h
+++ clang/lib/Headers/stddef.h
@@ -11,6 +11,17 @@
 defined(__need_size_t) || defined(__need_wchar_t) ||   \
 defined(__need_NULL) || defined(__need_wint_t)
 
+#if defined(__MVS__) && __has_include_next()
+#define __STDDEF_H
+#undef __need_ptrdiff_t
+#undef __need_size_t
+#undef __need_wchar_t
+#undef __need_NULL
+#undef __need_wint_t
+#include_next 
+
+#else
+
 #if !defined(__need_ptrdiff_t) && !defined(__need_size_t) &&   \
 !defined(__need_wchar_t) && !defined(__need_NULL) &&   \
 !defined(__need_wint_t)
@@ -130,4 +141,5 @@
 #undef __need_wint_t
 #endif /* __need_wint_t */
 
+#endif /* __MVS__ */
 #endif
Index: clang/lib/Headers/stdbool.h
===
--- clang/lib/Headers/stdbool.h
+++ clang/lib/Headers/stdbool.h
@@ -12,6 +12,10 @@
 
 #define __bool_true_false_are_defined 1
 
+#if defined(__MVS__) && __has_include_next()
+#include_next 
+#else
+
 #if defined(__STDC_VERSION__) && __STDC_VERSION__ > 201710L
 /* FIXME: We should be issuing a deprecation warning here, but cannot yet due
  * to system headers which include this header file unconditionally.
@@ -31,4 +35,5 @@
 #endif
 #endif
 
+#endif /* __MVS__ */
 #endif /* __STDBOOL_H */
Index: clang/lib/Headers/stdarg.h
===
--- clang/lib/Headers/stdarg.h
+++ clang/lib/Headers/stdarg.h
@@ -8,6 +8,10 @@
  */
 
 #ifndef __STDARG_H
+#if defined(__MVS__) && __has_include_next()
+#define __STDARG_H
+#include_next 
+#else
 
 #ifndef __GNUC_VA_LIST
 #define __GNUC_VA_LIST
@@ -46,6 

[PATCH] D155131: [clang][modules] Deserialize included files lazily

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 540174.
jansvoboda11 added a comment.
Herald added a subscriber: mgrang.

Remove dead code, make sure `getFileInfo()` is called in `alreadyIncluded()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155131

Files:
  clang/include/clang/Lex/Preprocessor.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/ASTReader.h
  clang/include/clang/Serialization/ASTWriter.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderInternals.h
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -866,7 +866,6 @@
   RECORD(CUDA_PRAGMA_FORCE_HOST_DEVICE_DEPTH);
   RECORD(PP_CONDITIONAL_STACK);
   RECORD(DECLS_TO_CHECK_FOR_DEFERRED_DIAGS);
-  RECORD(PP_INCLUDED_FILES);
   RECORD(PP_ASSUME_NONNULL_LOC);
 
   // SourceManager Block.
@@ -1763,6 +1762,7 @@
 
 struct data_type {
   const HeaderFileInfo 
+  bool AlreadyIncluded;
   ArrayRef KnownHeaders;
   UnresolvedModule Unresolved;
 };
@@ -1808,7 +1808,8 @@
   endian::Writer LE(Out, little);
   uint64_t Start = Out.tell(); (void)Start;
 
-  unsigned char Flags = (Data.HFI.isImport << 5)
+  unsigned char Flags = (Data.AlreadyIncluded << 6)
+  | (Data.HFI.isImport << 5)
   | (Data.HFI.isPragmaOnce << 4)
   | (Data.HFI.DirInfo << 1)
   | Data.HFI.IndexHeaderMapHeader;
@@ -1909,7 +1910,7 @@
 HeaderFileInfoTrait::key_type Key = {
 FilenameDup, *U.Size, IncludeTimestamps ? *U.ModTime : 0};
 HeaderFileInfoTrait::data_type Data = {
-Empty, {}, {M, ModuleMap::headerKindToRole(U.Kind)}};
+Empty, false, {}, {M, ModuleMap::headerKindToRole(U.Kind)}};
 // FIXME: Deal with cases where there are multiple unresolved header
 // directives in different submodules for the same header.
 Generator.insert(Key, Data, GeneratorTrait);
@@ -1952,11 +1953,13 @@
   SavedStrings.push_back(Filename.data());
 }
 
+bool Included = PP->alreadyIncluded(File);
+
 HeaderFileInfoTrait::key_type Key = {
   Filename, File->getSize(), getTimestampForOutput(File)
 };
 HeaderFileInfoTrait::data_type Data = {
-  *HFI, HS.getModuleMap().findResolvedModulesForHeader(File), {}
+  *HFI, Included, HS.getModuleMap().findResolvedModulesForHeader(File), {}
 };
 Generator.insert(Key, Data, GeneratorTrait);
 ++NumHeaderSearchEntries;
@@ -2262,29 +2265,6 @@
   return false;
 }
 
-void ASTWriter::writeIncludedFiles(raw_ostream , const Preprocessor ) {
-  using namespace llvm::support;
-
-  const Preprocessor::IncludedFilesSet  = PP.getIncludedFiles();
-
-  std::vector IncludedInputFileIDs;
-  IncludedInputFileIDs.reserve(IncludedFiles.size());
-
-  for (const FileEntry *File : IncludedFiles) {
-auto InputFileIt = InputFileIDs.find(File);
-if (InputFileIt == InputFileIDs.end())
-  continue;
-IncludedInputFileIDs.push_back(InputFileIt->second);
-  }
-
-  llvm::sort(IncludedInputFileIDs);
-
-  endian::Writer LE(Out, little);
-  LE.write(IncludedInputFileIDs.size());
-  for (uint32_t ID : IncludedInputFileIDs)
-LE.write(ID);
-}
-
 /// Writes the block containing the serialized form of the
 /// preprocessor.
 void ASTWriter::WritePreprocessor(const Preprocessor , bool IsModule) {
@@ -2533,20 +2513,6 @@
MacroOffsetsBase - ASTBlockStartOffset};
 Stream.EmitRecordWithBlob(MacroOffsetAbbrev, Record, bytes(MacroOffsets));
   }
-
-  {
-auto Abbrev = std::make_shared();
-Abbrev->Add(BitCodeAbbrevOp(PP_INCLUDED_FILES));
-Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob));
-unsigned IncludedFilesAbbrev = Stream.EmitAbbrev(std::move(Abbrev));
-
-SmallString<2048> Buffer;
-raw_svector_ostream Out(Buffer);
-writeIncludedFiles(Out, PP);
-RecordData::value_type Record[] = {PP_INCLUDED_FILES};
-Stream.EmitRecordWithBlob(IncludedFilesAbbrev, Record, Buffer.data(),
-  Buffer.size());
-  }
 }
 
 void ASTWriter::WritePreprocessorDetail(PreprocessingRecord ,
Index: clang/lib/Serialization/ASTReaderInternals.h
===
--- clang/lib/Serialization/ASTReaderInternals.h
+++ clang/lib/Serialization/ASTReaderInternals.h
@@ -276,6 +276,9 @@
   static internal_key_type ReadKey(const unsigned char *d, unsigned);
 
   data_type ReadData(internal_key_ref,const unsigned char *d, unsigned DataLen);
+
+private:
+  const FileEntry *getFile(const internal_key_type );
 };
 
 /// The on-disk hash table used for known header files.
Index: 

[PATCH] D154531: [AMDGPU] Support -mcpu=native for OpenCL

[Yaxun Liu 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 rG91b9bdeb9256: [AMDGPU] Support -mcpu=native for OpenCL 
(authored by yaxunl).
Herald added a subscriber: ldrumm.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154531

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/Driver/ToolChains/AMDGPU.cpp


Index: clang/lib/Driver/ToolChains/AMDGPU.cpp
===
--- clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -635,6 +635,27 @@
   for (Arg *A : Args)
 DAL->append(A);
 
+  // Replace -mcpu=native with detected GPU.
+  Arg *LastMCPUArg = DAL->getLastArg(options::OPT_mcpu_EQ);
+  if (LastMCPUArg && StringRef(LastMCPUArg->getValue()) == "native") {
+DAL->eraseArg(options::OPT_mcpu_EQ);
+auto GPUsOrErr = getSystemGPUArchs(Args);
+if (!GPUsOrErr) {
+  getDriver().Diag(diag::err_drv_undetermined_gpu_arch)
+  << llvm::Triple::getArchTypeName(getArch())
+  << llvm::toString(GPUsOrErr.takeError()) << "-mcpu";
+} else {
+  auto  = *GPUsOrErr;
+  if (GPUs.size() > 1) {
+getDriver().Diag(diag::warn_drv_multi_gpu_arch)
+<< llvm::Triple::getArchTypeName(getArch())
+<< llvm::join(GPUs, ", ") << "-mcpu";
+  }
+  DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_mcpu_EQ),
+Args.MakeArgString(GPUs.front()));
+}
+  }
+
   checkTargetID(*DAL);
 
   if (!Args.getLastArgValue(options::OPT_x).equals("cl"))
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -1336,6 +1336,9 @@
 // Warning about mixed HIP and OpenMP compilation / target offloading.
 def HIPOpenMPOffloading: DiagGroup<"hip-omp-target-directives">;
 
+// Warning about multiple GPUs are detected.
+def MultiGPU: DiagGroup<"multi-gpu">;
+
 // Warnings which cause linking of the runtime libraries like
 // libc and the CRT to be skipped.
 def AVRRtlibLinkingQuirks : DiagGroup<"avr-rtlib-linking-quirks">;
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -82,6 +82,9 @@
 def err_drv_undetermined_gpu_arch : Error<
   "cannot determine %0 architecture: %1; consider passing it via "
   "'%2'">;
+def warn_drv_multi_gpu_arch : Warning<
+  "multiple %0 architectures are detected: %1; only the first one is used for "
+  "'%2'">, InGroup;
 def err_drv_cuda_version_unsupported : Error<
   "GPU arch %0 is supported by CUDA versions between %1 and %2 (inclusive), "
   "but installation at %3 is %4; use '--cuda-path' to specify a different CUDA 
"


Index: clang/lib/Driver/ToolChains/AMDGPU.cpp
===
--- clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -635,6 +635,27 @@
   for (Arg *A : Args)
 DAL->append(A);
 
+  // Replace -mcpu=native with detected GPU.
+  Arg *LastMCPUArg = DAL->getLastArg(options::OPT_mcpu_EQ);
+  if (LastMCPUArg && StringRef(LastMCPUArg->getValue()) == "native") {
+DAL->eraseArg(options::OPT_mcpu_EQ);
+auto GPUsOrErr = getSystemGPUArchs(Args);
+if (!GPUsOrErr) {
+  getDriver().Diag(diag::err_drv_undetermined_gpu_arch)
+  << llvm::Triple::getArchTypeName(getArch())
+  << llvm::toString(GPUsOrErr.takeError()) << "-mcpu";
+} else {
+  auto  = *GPUsOrErr;
+  if (GPUs.size() > 1) {
+getDriver().Diag(diag::warn_drv_multi_gpu_arch)
+<< llvm::Triple::getArchTypeName(getArch())
+<< llvm::join(GPUs, ", ") << "-mcpu";
+  }
+  DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_mcpu_EQ),
+Args.MakeArgString(GPUs.front()));
+}
+  }
+
   checkTargetID(*DAL);
 
   if (!Args.getLastArgValue(options::OPT_x).equals("cl"))
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -1336,6 +1336,9 @@
 // Warning about mixed HIP and OpenMP compilation / target offloading.
 def HIPOpenMPOffloading: DiagGroup<"hip-omp-target-directives">;
 
+// Warning about multiple GPUs are detected.
+def MultiGPU: DiagGroup<"multi-gpu">;
+
 // Warnings which cause linking of the runtime libraries like
 // libc and the CRT to be skipped.
 def AVRRtlibLinkingQuirks : 

[clang] 91b9bde - [AMDGPU] Support -mcpu=native for OpenCL

[Yaxun Liu via cfe-commits]

Author: Yaxun (Sam) Liu
Date: 2023-07-13T16:21:35-04:00
New Revision: 91b9bdeb92562d3030d834489185a10aa8e241ef

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

LOG: [AMDGPU] Support -mcpu=native for OpenCL

When -mcpu=native is specified, try detecting GPU
on the system by using amdgpu-arch tool. If it
fails to detect GPU, emit an error about GPU
not detected. If multiple GPUs are detected,
use the first GPU and emit a warning.

Reviewed by: Matt Arsenault, Fangrui Song

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

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticDriverKinds.td
clang/include/clang/Basic/DiagnosticGroups.td
clang/lib/Driver/ToolChains/AMDGPU.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 095fbfe8ba2b1d..d10262826bf292 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -82,6 +82,9 @@ def err_drv_hipspv_no_hip_path : Error<
 def err_drv_undetermined_gpu_arch : Error<
   "cannot determine %0 architecture: %1; consider passing it via "
   "'%2'">;
+def warn_drv_multi_gpu_arch : Warning<
+  "multiple %0 architectures are detected: %1; only the first one is used for "
+  "'%2'">, InGroup;
 def err_drv_cuda_version_unsupported : Error<
   "GPU arch %0 is supported by CUDA versions between %1 and %2 (inclusive), "
   "but installation at %3 is %4; use '--cuda-path' to specify a 
diff erent CUDA "

diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index 8b15337fda8829..cfc1c9bc15bd2d 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -1336,6 +1336,9 @@ def HIPOnly : DiagGroup<"hip-only">;
 // Warning about mixed HIP and OpenMP compilation / target offloading.
 def HIPOpenMPOffloading: DiagGroup<"hip-omp-target-directives">;
 
+// Warning about multiple GPUs are detected.
+def MultiGPU: DiagGroup<"multi-gpu">;
+
 // Warnings which cause linking of the runtime libraries like
 // libc and the CRT to be skipped.
 def AVRRtlibLinkingQuirks : DiagGroup<"avr-rtlib-linking-quirks">;

diff  --git a/clang/lib/Driver/ToolChains/AMDGPU.cpp 
b/clang/lib/Driver/ToolChains/AMDGPU.cpp
index 2a5b4b66275ff6..d0223322b56ba4 100644
--- a/clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ b/clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -635,6 +635,27 @@ AMDGPUToolChain::TranslateArgs(const DerivedArgList , 
StringRef BoundArch,
   for (Arg *A : Args)
 DAL->append(A);
 
+  // Replace -mcpu=native with detected GPU.
+  Arg *LastMCPUArg = DAL->getLastArg(options::OPT_mcpu_EQ);
+  if (LastMCPUArg && StringRef(LastMCPUArg->getValue()) == "native") {
+DAL->eraseArg(options::OPT_mcpu_EQ);
+auto GPUsOrErr = getSystemGPUArchs(Args);
+if (!GPUsOrErr) {
+  getDriver().Diag(diag::err_drv_undetermined_gpu_arch)
+  << llvm::Triple::getArchTypeName(getArch())
+  << llvm::toString(GPUsOrErr.takeError()) << "-mcpu";
+} else {
+  auto  = *GPUsOrErr;
+  if (GPUs.size() > 1) {
+getDriver().Diag(diag::warn_drv_multi_gpu_arch)
+<< llvm::Triple::getArchTypeName(getArch())
+<< llvm::join(GPUs, ", ") << "-mcpu";
+  }
+  DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_mcpu_EQ),
+Args.MakeArgString(GPUs.front()));
+}
+  }
+
   checkTargetID(*DAL);
 
   if (!Args.getLastArgValue(options::OPT_x).equals("cl"))



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

[PATCH] D148654: Modify BoundsSan to improve debuggability

[Oskar Wirga via Phabricator via cfe-commits]

oskarwirga added inline comments.



Comment at: clang/lib/CodeGen/CGExpr.cpp:3576-3597
+if (TrapBBs.size() <= CheckHandlerID)
+  TrapBBs.resize(CheckHandlerID + 1);
+llvm::BasicBlock * = TrapBBs[CheckHandlerID];
+
+if (!CGM.getCodeGenOpts().OptimizationLevel || !TrapBB ||
+(CurCodeDecl && CurCodeDecl->hasAttr())) {
+  TrapBB = createBasicBlock("trap");

vitalybuka wrote:
> looks like a lot of code duplication
Let me refactor this 



Comment at: clang/lib/CodeGen/CGExpr.cpp:3594
+TrapCall->addFnAttr(A);
+  }
+  TrapCall->setDoesNotReturn();

vitalybuka wrote:
> wouldn't be you issues solved with
> TrapCall->setCannotMerge() here?
ubsantrap gets lowered to an instruction in MIR which then gets merged later. 
This was the only way I was able to create unique instruction per check. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148654

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

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86-64 datalayout

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

In D86310#4498721 , @hvdijk wrote:

> In D86310#4498575 , @efriedma wrote:
>
>> https://reviews.llvm.org/D86310#2231136 has an example where IR generated by 
>> clang breaks.
>
> clang bases it on the data layout, so when the change here is applied, clang 
> already generates correct IR for that example without further changes (using 
> `%struct.Y = type <{ i64, %struct.X }>`). Unless you were using that as an 
> example of when using old clang to generate LLVM IR, and new LLVM to produce 
> machine code, would break?

I only meant to dispute the assertion that ABI compatibility with old IR is 
only a problem for non-clang frontends.


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

https://reviews.llvm.org/D86310

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

[PATCH] D154881: [HIP] Ignore host linker flags for device-only

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: clang/test/Driver/hip-phases.hip:275
 // RELOC-DAG: [[P5:[0-9]+]]: offload, "device-[[T]] 
(amdgcn-amd-amdhsa:[[ARCH]])" {[[P4]]}, object
 
 //

need to check RELOC-NOT: host



Comment at: clang/test/Driver/hip-phases.hip:302
 // RELOC2-DAG: [[P11:[0-9]+]]: offload, "device-[[T]] 
(amdgcn-amd-amdhsa:[[ARCH2]])" {[[P10]]}, object
 
 //

need to check RELOC2-NOT: host



Comment at: clang/test/Driver/hip-phases.hip:422
 // RL2-DEV-NOT: linker
 
 // Test one gpu architectures up to the preprocessor expansion output phase in 
device-only

need to check RL2-DEV-NOT: host


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154881

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

[PATCH] D153058: [clang][CFG] Support construction of a weak topological ordering of the CFG.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Having (mostly!) understood what this is doing, the algorithm looks like it's 
doing the right thing.

High-level ideas would be:

- there are three different representations/sets of APIs exposed: intervals, 
ordering, and compare. It seems like we only have immediate plans to use 
compare, and maybe ordering in the future. Maybe we could hide the stuff we're 
not using, if it's necessary to test it then a narrower interface for testing 
might be enough? Like `string printIntervalGraph(CFG, int iters)` and then all 
of Interval could be hidden.
- the data structures for the interval graph are mechanically complicated 
(trees with differently templated node types etc), and dealing with this 
incidental complexity and the algorithm complexity is a lot. It may be possible 
to use simpler data structures, as we don't make use of everything in these 
ones.




Comment at: clang/include/clang/Analysis/Analyses/IntervalPartition.h:38
+// interval.
 struct CFGInterval {
+  CFGInterval() = default;

Summarizing an offline discussion, IIUC...
- we build a tree of intervals that represents the whole sequence of interval 
graphs
- for the current worklist algorithm, we use only the total order and process 
the first invalid block, the tree is not needed
- the algorithm/data structure could be significantly simplified (no variants, 
templates, recursion...) if it didn't build the tree, just the interval graphs 
iteratively until a single node is reached
- however a different, non-worklist algorithm could use the tree information to 
determine which blocks to visit next. (Given `a (b c d) e`, after `d` we try 
only `b` or `e`). This avoids expensive "is invalid" checks
- so the motivation for preserving the full interval tree is future-proofing 
for using a different algorithm

I think it's worth documenting this motivation, and also considering not paying 
this complexity cost for a feature we aren't adding yet.



Comment at: clang/include/clang/Analysis/Analyses/IntervalPartition.h:56
+
+  template  struct NodeData {
+// The block from which the interval was constructed. It is either the

xazax.hun wrote:
> Correct me if I'm wrong but I have the impression that `CFGInterval` might 
> either contain only `CfgBlock`s or other `CfgInterval`s, but these might 
> never be mixed. The current representation does allow such mixing. In case do 
> not need that, I think it might be cleaner to make `CfgInterval` itself 
> templated and remove the `std::variant`.
(Very optional idea, in case you like it)

This data structure is complicated: a tree of Intervals with CFGBlocks at the 
root, and at every level the Intervals have pred/succ edges that form part of 
one of the intermediate flow graphs.

It seems nice to get rid of the tree if possible (ultimately we're using these 
for ordering) and the nested pred/succs - we seem to only need these for the 
top level interval (and in the end, we have one interval and pred/succs are 
empty.

What if we made this a flat structure, preserving the bracket structure as some 
kind of back-edge?

```
1  (  2  (  3  4  )  )  5
becomes
1 2 3  4 !3 !2  5
```

AIUI, the dataflow algorithm can basically treat this as a program, we move 
left to right and `!X` means "if (X is invalidated) goto X".

This would give us something like:
```
using Entry = PointerIntPair; // either an actual block, or a 
conditional jump
struct Interval { vector Sequence; SmallDenseSet Preds, 
Succs; }
using IntervalGraph = vector;
```

I also suspect that this would let us avoid the core algorithms (buildInterval, 
addIntervalNode) being templated, simply wrap each CFGBlock in a trivial 
Interval to start with instead. I guess this is OK performance-wise, but the 
reason not to do it is ending up with those nodes polluting your final data 
structure?


Anyway, all this is up to you - for me keeping more data/structure around than 
we're going to use is confusing, but it does seem closer to the textbook.



Comment at: clang/include/clang/Analysis/Analyses/IntervalPartition.h:123
+/// This WTO construction is described in Section 4.2 of [Bourdoncle1993].
+std::optional getIntervalWTO(const CFG );
+

Naming is confusing here: `getWTO` is the internal function whose API involves 
intervals, and `getIntervalWTO` be the main function whose API doesn't. I think 
literally swapping them might be clearer!

in general it's hard to tell which parts of this API people are actually meant 
to use.
Is it reasonable to wrap everything up to `WeakTopologicalOrdering` in 
`namespace internal` or so? Or are there places this will be used outside tests?



Comment at: clang/include/clang/Analysis/Analyses/IntervalPartition.h:126
+/// Formats `WTO` as a human-readable string.
+std::string formatWTO(const WeakTopologicalOrdering );
+

consider instead exposing `printWTO` 

[PATCH] D133289: [C2X] N3007 Type inference for object definitions

[Guillot Tony via Phabricator via cfe-commits]

to268 marked an inline comment as done.
to268 added a comment.

Thank you for your feedback @aaron.ballman!
I really appreciate that you are pointing out all my formatting mistakes, and 
giving me more guidelines.
The direction of the patch is clearer now.

In D133289#4489883 , @aaron.ballman 
wrote:

> I think there's a typo in the patch summary:
>
>> auto in an compound statement
>
> I think you mean "as the type of a compound literal"?

Yes i was a mistake, I have edited the summary to fix that typo.

In D133289#4489883 , @aaron.ballman 
wrote:

> - We should have an extension warning for array use with string literals 
> `auto str[] = "testing";`

This makes sense, since auto arrays are prohibited in the standard.

In D133289#4497901 , @aaron.ballman 
wrote:

> The committee is discussing this again on the reflectors. Thus far, almost 
> everyone reads the standard the same way as GCC did with their 
> implementation, which matches what I suggest above. However, there are folks 
> who are claiming we should not be able to deduce the derived type because 
> `_Atomic` forms an entirely new type and thus isn't actually a qualifier (and 
> there are some words in the standard that could maybe be read as supporting 
> that). The most conservative approach is to reject using `_Atomic auto` for 
> right now so users don't build a reliance on it. Eventually WG14 will make a 
> decision and we can relax that diagnostic then if we need to. Sorry for the 
> confusion on this topic!

I was wondering about the support of `_Atomic auto`, i will add new error 
diagnostic to prohibit `_Atomic auto`, thank you for addressing that topic!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133289

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

[PATCH] D155175: [Clang] Fix consteval propagation for aggregates and defaulted constructors

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Do we need CodeGen testcases here for full coverage?  The testcases from the 
issue passed with -fsyntax-only.



With this patch, the following cases produce errors that don't really make 
sense to me:

  consteval int f(int x) {
  return x;
  }
  struct SS {
  int y;
  int x = f(this->y);
  constexpr SS(int yy) : y(yy) {}
  };
  SS s = {1};



  :6:13: error: call to consteval function 'f' is not a constant 
expression
  6 | int x = f(this->y);
| ^
  :7:15: note: in the default initializer of 'x'
  7 | constexpr SS(int yy) : y(yy) {}
|   ^
  :6:5: note: declared here
  6 | int x = f(this->y);
| ^
  :6:15: note: use of 'this' pointer is only allowed within the 
evaluation of a call to a 'constexpr' member function
  6 | int x = f(this->y);
|   ^
  1 error generated.



  consteval int f(int x) {
  return x;
  }
  struct SS {
  int y;
  int x = f(this->y);
  consteval SS(int yy) : y(yy) {}
  };
  SS s = {1};

  :6:13: error: cannot take address of consteval function 'f' outside of 
an immediate invocation
  6 | int x = f(this->y);
| ^
  :1:15: note: declared here
  1 | consteval int f(int x) {
|   ^
  1 error generated.

Somehow the following is accepted, though:

  consteval int f(int x) {
  return x;
  }
  struct SS {
  int y;
  int x = f(this->y);
  template constexpr SS(T yy) : y(yy) {}
  };
  SS s = {1};


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155175

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

[PATCH] D154869: [Flang] [FlangRT] Implement FlangRT library as solution to Flang's runtime LLVM integration

[Paul Scoropan via Phabricator via cfe-commits]

pscoro added inline comments.



Comment at: flang-rt/CMakeLists.txt:17-23
+# Check if flang-rt is built as a standalone project.
+if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR OR 
FLANG_RT_STANDALONE_BUILD)
+  project(FlangRT C CXX)
+  set(CMAKE_INCLUDE_CURRENT_DIR ON)
+  set(FLANG_RT_STANDALONE_BUILD TRUE)
+  set_property(GLOBAL PROPERTY USE_FOLDERS ON)
+endif()

vzakhari wrote:
> phosek wrote:
> > I don't think we should support building FlangRT as a standalone project. 
> > We've been moving all runtimes to the runtimes build, the only leftover is 
> > compiler-rt and our hope is to address that soon.
> Hi @phosek, can you please explain what you mean by "We've been moving all 
> runtimes to the runtimes build"?  Can you please share references to the 
> pipeline for the "runtimes build"?
If i understood correctly, i think @phosek is talking about building the 
flang-rt target directly (`cmake llvm-project/flang-rt`) when he says 
standalone. Meanwhile a runtimes build is `cmake llvm-project/runtimes 
-DLLVM_ENABLE_RUNTIMES=flang-rt`. I chose not to change anything yet because I 
was focused on some other bugs and was waiting for see if anybody else had 
input on this first. Also, in case my understanding is incorrect @phosek please 
correct me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154869

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

[PATCH] D154869: [Flang] [FlangRT] Implement FlangRT library as solution to Flang's runtime LLVM integration

[Slava Zakharin via Phabricator via cfe-commits]

vzakhari added inline comments.



Comment at: flang-rt/CMakeLists.txt:17-23
+# Check if flang-rt is built as a standalone project.
+if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR OR 
FLANG_RT_STANDALONE_BUILD)
+  project(FlangRT C CXX)
+  set(CMAKE_INCLUDE_CURRENT_DIR ON)
+  set(FLANG_RT_STANDALONE_BUILD TRUE)
+  set_property(GLOBAL PROPERTY USE_FOLDERS ON)
+endif()

phosek wrote:
> I don't think we should support building FlangRT as a standalone project. 
> We've been moving all runtimes to the runtimes build, the only leftover is 
> compiler-rt and our hope is to address that soon.
Hi @phosek, can you please explain what you mean by "We've been moving all 
runtimes to the runtimes build"?  Can you please share references to the 
pipeline for the "runtimes build"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154869

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

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86-64 datalayout

[Harald van Dijk via Phabricator via cfe-commits]

hvdijk added a comment.

In D86310#4498575 , @efriedma wrote:

> https://reviews.llvm.org/D86310#2231136 has an example where IR generated by 
> clang breaks.

clang bases it on the data layout, so when the change here is applied, clang 
already generates correct IR for that example without further changes (using 
`%struct.Y = type <{ i64, %struct.X }>`).


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

https://reviews.llvm.org/D86310

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

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86-64 datalayout

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> The only problem that approach 2 solves is to ensure that a non-clang 
> frontend using i128

https://reviews.llvm.org/D86310#2231136 has an example where IR generated by 
clang breaks.


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

https://reviews.llvm.org/D86310

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

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86-64 datalayout

[Harald van Dijk via Phabricator via cfe-commits]

hvdijk added a comment.

In D86310#4498551 , @rnk wrote:

> Given that most non-clang frontends want the bug fix (ABI break), who exactly 
> is asking for this level of IR ABI stability?

You were, I thought, or at least that's how I interpreted your earlier comment. 
:) If we're now all in agreement that that level of ABI stability is not 
needed, I can update this patch to address the comment that I had left (it 
should not be limited to 64-bit, it's needed for all X86). I'll probably be 
able to find time for this in the weekend.


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

https://reviews.llvm.org/D86310

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

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86-64 datalayout

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

I see two ways forward here:

1. Autoupgrade modules with old datalayout strings by increasing the alignment 
of i128 & co. This will change LLVM IR struct layouts, argument alignments, 
etc. As far as native ABI boundaries are concerned, this should be "more 
correct": Clang explicitly applies `alignstack` attributes to increase the 
alignment of i128 arguments, and adds padding to structs to align i128. As far 
as IR ABI boundaries within LTO are concerned, it is ABI compatible with IR 
modules.
2. Freeze the ABI of the old module during autoupgrade. Replace all struct 
types with equivalent packed structs and explicit padding. Apply explicit 
alignments to all i128 loads and stores. Apply explicit `alignstack(8)` 
attributes to all i128 arguments.

I think 1 is better than 2. The only problem that approach 2 solves is to 
ensure that a non-clang frontend using i128 is ABI compatible with old versions 
of that same frontend (think Rust). Given that most non-clang frontends want 
the bug fix (ABI break), who exactly is asking for this level of IR ABI 
stability? Maybe I'm missing something, but after skimming over this review 
again, I think the existing autoupgrade approach is probably good enough. Can 
we add a release note or something and leave it at that?


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

https://reviews.llvm.org/D86310

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

[PATCH] D148654: Modify BoundsSan to improve debuggability

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added inline comments.



Comment at: clang/lib/CodeGen/CGExpr.cpp:3576-3597
+if (TrapBBs.size() <= CheckHandlerID)
+  TrapBBs.resize(CheckHandlerID + 1);
+llvm::BasicBlock * = TrapBBs[CheckHandlerID];
+
+if (!CGM.getCodeGenOpts().OptimizationLevel || !TrapBB ||
+(CurCodeDecl && CurCodeDecl->hasAttr())) {
+  TrapBB = createBasicBlock("trap");

looks like a lot of code duplication



Comment at: clang/lib/CodeGen/CGExpr.cpp:3594
+TrapCall->addFnAttr(A);
+  }
+  TrapCall->setDoesNotReturn();

wouldn't be you issues solved with
TrapCall->setCannotMerge() here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148654

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

[PATCH] D152770: [clang][ExtractAPI] Add support for Objective-C categories

[Zixu Wang via Phabricator via cfe-commits]

zixuw added a comment.

Thanks for the patch!

Like I mentioned in the inline comments, I'm not sure I understand the need of 
the "abstract" (in the sense that it's not really a concrete API) subtype of 
`APIRecord`. The design doesn't feel right within the existing structure.
If I recall the original issue correctly the problem is that a category might 
extend an interface that is not declared within the current compilation unit 
and the SymbolGraph output has references to an USR it cannot resolve. But 
that's why we have the `SymbolReference Interface` for the category record, and 
all the information we could possibly have from extract-api for that interface 
is already in the `SymbolReference`. Couldn't we use that to design an output 
format to indicate the external reference?

Also a side note that the word "module" has been overloaded, especially within 
LLVM/clang where "module" is used for clang modules or C++ modules. I would 
avoid creating a `*ModuleRecord` without qualifying it like `*ExtractAPIModule` 
and providing documentation to clarify the meaning.




Comment at: clang/include/clang/ExtractAPI/API.h:148-150
+  APIRecord(RecordKind Kind, StringRef USR, StringRef Name)
+  : USR(USR), Name(Name), Kind(Kind) {}
+

I would really try to avoid patching the base `APIRecord` to get ways of 
creating "fake" records. The API records in the API set should really be all 
concrete symbols mapping back to an entity that is an API.
This doesn't feel right and might open up to future bad designs and bugs.



Comment at: clang/include/clang/ExtractAPI/API.h:497-510
+struct ObjCCategoryModuleRecord : APIRecord {
+  // ObjCCategoryRecord%s are stored in and owned by APISet.
+  SmallVector Categories;
+
+  ObjCCategoryModuleRecord(StringRef USR, StringRef Name)
+  : APIRecord(RK_ObjCCategoryModule, USR, Name) {}
+

I'm still not convinced by this concept and design. Why do we need to have a 
subtype of `APIRecord` to represent relationships with external types from 
another "module"? We have `SymbolReference` for that purpose with the name and 
USR information.

Even if this is necessary, the name is extremely confusing. By the look of the 
rest of the change and the test cases, I believe this is meant to represent the 
external *interface* that a category is extending, not a category.



Comment at: clang/include/clang/ExtractAPI/ExtractAPIVisitor.h:128-135
+template 
+static bool isExternalCategory(const T , const StringRef ) {
+  for (const auto  : Records) {
+if (Name == Record.second.get()->Name)
+  return false;
+  }
+  return true;

This looks very concerning..
It's way too generic for its name and it's not doing what it's describing.
By the look of the call site below, this seems to be supposed to check if an 
*interface*, not category, is recorded in this API set. There's no reason to 
have a separate template function for that. You can just do the check inline 
and also probably covered by one of the existing llvm STLExtra utilities.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152770

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

[PATCH] D155146: Add SHA512 instructions.

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: llvm/lib/Target/X86/X86.td:243
+  "Support SHA512 instructions",
+  [FeatureAVX]>;
 // Processor supports CET SHSTK - Control-Flow Enforcement Technology

AVX2 like other integer features?



Comment at: llvm/lib/Target/X86/X86InstrSSE.td:8304
+[(set VR256:$dst,
+ (int_x86_vsha512msg1 VR256:$src1, VR128:$src2))]>, VEX_L,
+VEX, T8XD, Sched<[WriteVecIMul]>;

This paren should be indented 1 more space so that it's not at the same column 
as the one above it.



Comment at: llvm/lib/Target/X86/X86InstrSSE.td:8310
+[(set VR256:$dst,
+ (int_x86_vsha512msg2 VR256:$src1, VR256:$src2))]>, VEX_L,
+VEX, T8XD, Sched<[WriteVecIMul]>;

ditto



Comment at: llvm/lib/Target/X86/X86InstrSSE.td:8316
+  [(set VR256:$dst,
+   (int_x86_vsha512rnds2 VR256:$src1, VR256:$src2, 
VR128:$src3))]>,
+  VEX_L, VEX_4V, T8XD, Sched<[WriteVecIMul]>;

ditto



Comment at: llvm/lib/TargetParser/X86TargetParser.cpp:214
 FeaturesSapphireRapids | FeatureAMX_FP16 | FeaturePREFETCHI |
+FeatureSHA512 |
 FeatureAMX_COMPLEX;

Unnecessary line break



Comment at: llvm/lib/TargetParser/X86TargetParser.cpp:659
 constexpr FeatureBitset ImpliedFeaturesAVXNECONVERT = FeatureAVX2;
+constexpr FeatureBitset ImpliedFeaturesSHA512 = FeatureAVX;
 constexpr FeatureBitset ImpliedFeaturesAVX512FP16 =

Should this be AVX2 like all the other integer features?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155146

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

[clang] d74421a - Remove Clang :: CodeGenCXX/unified-cfi-lto.cpp due to buildbot failures

[Matthew Voss via cfe-commits]

Author: Matthew Voss
Date: 2023-07-13T11:04:32-07:00
New Revision: d74421a29040d728e43f38ffa003d6cc22fbd0c6

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

LOG: Remove Clang :: CodeGenCXX/unified-cfi-lto.cpp due to buildbot failures

This test has been failing on sanitizer-x86_64-linux-bootstrap-asan
since it was commited. Removing this test while I work on reproducing
this.

Example: https://lab.llvm.org/buildbot/#/builders/168/builds/14579

Added: 


Modified: 


Removed: 
clang/test/CodeGenCXX/unified-cfi-lto.cpp



diff  --git a/clang/test/CodeGenCXX/unified-cfi-lto.cpp 
b/clang/test/CodeGenCXX/unified-cfi-lto.cpp
deleted file mode 100644
index 2c518e8e014c44..00
--- a/clang/test/CodeGenCXX/unified-cfi-lto.cpp
+++ /dev/null
@@ -1,22 +0,0 @@
-// Ensure that the frontend adds the proper metadata when CFI is
-// enabled.
-// RUN: %clang --target=x86_64-scei-ps4 -funified-lto -flto -fsanitize=cfi 
-fvisibility=hidden -fno-sanitize-ignorelist -c %s -o %t.o
-// RUN: llvm-dis %t.o -o %t1
-// RUN: FileCheck <%t1.0 %s
-
-typedef int (*FuncPtr)();
-
-int a() { return 1; }
-int b() { return 2; }
-int c() { return 3; }
-
-FuncPtr func[3] = {a,b,c};
-
-int
-main(int argc, char *argv[]) {
-  // CHECK: call i1 @llvm.type.test
-  return func[argc]();
-  // CHECK-LABEL: trap
-}
-
-// CHECK: typeTests:



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

[PATCH] D4784: [clang-tidy] Add check for possibly incomplete switch statements

[Shivam Gupta via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone/switch-missing-default-case.rst:52
+.. note::
+   Enum types are already covered by compiler warnings when a switch statement
+   does not handle all enum values. This check focuses on non-enum types where

PiotrZSL wrote:
> xgupta wrote:
> > PiotrZSL wrote:
> > > would be nice to list them
> > Didn't understand, what should I list, non-enum types?
> No, not a types, compiler warnings (ones for enums). Simply so when user 
> enable this check for integers, He/she could make sure that enum specific 
> warnings are also enabled.
> and remove this list that you added, as you cannot do switch on pointers or 
> floating point types.
I can see there is -Wswitch which I have mentioned.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D4784

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

[PATCH] D4784: [clang-tidy] Add check for possibly incomplete switch statements

[Shivam Gupta via Phabricator via cfe-commits]

xgupta updated this revision to Diff 540119.
xgupta marked an inline comment as done.
xgupta added a comment.

Address latest two comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D4784

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SwitchMissingDefaultCaseCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SwitchMissingDefaultCaseCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone/switch-missing-default-case.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/switch-missing-default-case.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/switch-missing-default-case.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/switch-missing-default-case.cpp
@@ -0,0 +1,67 @@
+// RUN: %check_clang_tidy %s bugprone-switch-missing-default-case -- %t
+
+typedef int MyInt;
+enum EnumType { eE2 };
+typedef EnumType MyEnum;
+
+void positive() {
+  int I1 = 0;
+  // CHECK-MESSAGES: [[@LINE+1]]:11: warning: switching on non-enum value without default case may not cover all cases [bugprone-switch-missing-default-case]
+  switch (I1) {
+  case 0:
+break;
+  }
+
+  MyInt I2 = 0;
+  // CHECK-MESSAGES: [[@LINE+1]]:11: warning: switching on non-enum value without default case may not cover all cases [bugprone-switch-missing-default-case]
+  switch (I2) {
+  case 0:
+break;
+  }
+
+  int getValue();
+  // CHECK-MESSAGES: [[@LINE+1]]:11: warning: switching on non-enum value without default case may not cover all cases [bugprone-switch-missing-default-case]
+  switch (getValue()) {
+  case 0:
+break;
+  }
+}
+
+void negative() {
+  enum E { eE1 };
+  E E1 = eE1;
+  switch (E1) { // no-warning
+  case eE1:
+break;
+  }
+
+  MyEnum E2 = eE2;
+  switch (E2) { // no-warning
+  case eE2:
+break;
+  }
+
+  int I1 = 0;
+  switch (I1) { // no-warning
+  case 0:
+break;
+  default:
+break;
+  }
+
+  MyInt I2 = 0;
+  switch (I2) { // no-warning
+  case 0:
+break;
+  default:
+break;
+  }
+
+  int getValue();
+  switch (getValue()) { // no-warning
+  case 0:
+break;
+default:
+break;
+  }
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -92,6 +92,7 @@
`bugprone-forwarding-reference-overload `_,
`bugprone-implicit-widening-of-multiplication-result `_, "Yes"
`bugprone-inaccurate-erase `_, "Yes"
+   `bugprone-switch-missing-default-case `_, "Yes"
`bugprone-incorrect-roundings `_,
`bugprone-infinite-loop `_,
`bugprone-integer-division `_,
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/switch-missing-default-case.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/switch-missing-default-case.rst
@@ -0,0 +1,56 @@
+.. title:: clang-tidy - bugprone-switch-missing-default-case
+
+bugprone-switch-missing-default-case
+
+
+Ensures that switch statements without default cases are flagged, focuses only
+on covering cases with non-enums where the compiler may not issue warnings.
+
+Switch statements without a default case can lead to unexpected
+behavior and incomplete handling of all possible cases. When a switch statement
+lacks a default case, if a value is encountered that does not match any of the
+specified cases, the program will continue execution without any defined
+behavior or handling.
+
+This check helps identify switch statements that are missing a default case,
+allowing developers to ensure that all possible cases are handled properly.
+Adding a default case allows for graceful handling of unexpected or unmatched
+values, reducing the risk of program errors and unexpected behavior.
+
+Example:
+
+.. code-block:: c++
+
+  // Example 1:
+  // warning: switching on non-enum value without default case may not cover all cases
+  switch (i) {
+  case 0:
+break;
+  }
+
+  // Example 2:
+  enum E { eE1 };
+  E e = eE1;
+  switch (e) { // no-warning
+  case eE1:
+break;
+  }
+
+  // Example 3:
+  int i = 0;
+  switch (i) { // no-warning
+  case 0:
+break;
+  default:
+break;
+  }
+
+.. note::
+   Enum types are already covered by compiler warnings (comes under -Wswitch)
+   when a switch statement does not handle all enum values. This check focuses
+   on non-enum types where the compiler warnings may not be present.
+
+.. seealso::
+   The `CppCoreGuideline ES.79 

[PATCH] D155222: [RISCV][AArch64][IRGen] Add scalable->fixed as a special case in CreateCoercedStore.

[Craig Topper via Phabricator via cfe-commits]

craig.topper created this revision.
craig.topper added reviewers: bsmith, sdesmalen, c-rhodes, joechrisellis.
Herald added subscribers: jobnoorman, luke, VincentWu, vkmr, frasercrmck, 
luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, 
PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, 
shiva0217, kito-cheng, niosHD, sabuasal, simoncook, johnrusso, rbar, asb, 
kristof.beyls, arichardson.
Herald added a project: All.
craig.topper requested review of this revision.
Herald added subscribers: wangpc, alextsao1999, eopXD.
Herald added a project: clang.

This improves the codegen for calling a function that returns a
VLST type. Previously we stored to an alloca using the scalable
type, but loaded it using the fixed vector type. The middle end is
unable to optimize away that store/load pair. With this patch we now
store using the fixed vector type which matches the load.

I have not added predicate types because I haven't supported
those on RISC-V yet so I haven't seen a problem yet.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155222

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CodeGen/attr-arm-sve-vector-bits-call.c
  clang/test/CodeGen/attr-riscv-rvv-vector-bits-call.c


Index: clang/test/CodeGen/attr-riscv-rvv-vector-bits-call.c
===
--- clang/test/CodeGen/attr-riscv-rvv-vector-bits-call.c
+++ clang/test/CodeGen/attr-riscv-rvv-vector-bits-call.c
@@ -38,11 +38,7 @@
 
 // CHECK-LABEL: @sizeless_caller(
 // CHECK-NEXT:  entry:
-// CHECK-NEXT:[[COERCE1:%.*]] = alloca <8 x i32>, align 8
-// CHECK-NEXT:store  [[X:%.*]], ptr [[COERCE1]], align 8
-// CHECK-NEXT:[[TMP0:%.*]] = load <8 x i32>, ptr [[COERCE1]], align 8, 
!tbaa [[TBAA4:![0-9]+]]
-// CHECK-NEXT:[[CASTSCALABLESVE2:%.*]] = tail call  
@llvm.vector.insert.nxv2i32.v8i32( undef, <8 x i32> [[TMP0]], 
i64 0)
-// CHECK-NEXT:ret  [[CASTSCALABLESVE2]]
+// CHECK-NEXT:ret  [[X:%.*]]
 //
 vint32m1_t sizeless_caller(vint32m1_t x) {
   return fixed_callee(x);
Index: clang/test/CodeGen/attr-arm-sve-vector-bits-call.c
===
--- clang/test/CodeGen/attr-arm-sve-vector-bits-call.c
+++ clang/test/CodeGen/attr-arm-sve-vector-bits-call.c
@@ -41,11 +41,7 @@
 
 // CHECK-LABEL: @sizeless_caller(
 // CHECK-NEXT:  entry:
-// CHECK-NEXT:[[COERCE1:%.*]] = alloca <16 x i32>, align 16
-// CHECK-NEXT:store  [[X:%.*]], ptr [[COERCE1]], align 16
-// CHECK-NEXT:[[TMP1:%.*]] = load <16 x i32>, ptr [[COERCE1]], align 16, 
!tbaa [[TBAA6:![0-9]+]]
-// CHECK-NEXT:[[CASTSCALABLESVE2:%.*]] = tail call  
@llvm.vector.insert.nxv4i32.v16i32( undef, <16 x i32> 
[[TMP1]], i64 0)
-// CHECK-NEXT:ret  [[CASTSCALABLESVE2]]
+// CHECK-NEXT:ret  [[X:%.*]]
 //
 svint32_t sizeless_caller(svint32_t x) {
   return fixed_callee(x);
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1390,6 +1390,19 @@
 return;
   }
 
+  // If coercing a fixed vector from a scalable vector for ABI compatibility,
+  // and the types match, use the llvm.vector.extract intrinsic to perform the
+  // conversion.
+  if (auto *FixedDst = dyn_cast(DstTy)) {
+if (auto *ScalableSrc = dyn_cast(SrcTy)) {
+  if (FixedDst->getElementType() == ScalableSrc->getElementType()) {
+auto *Zero = llvm::Constant::getNullValue(CGF.CGM.Int64Ty);
+Src = CGF.Builder.CreateExtractVector(DstTy, Src, Zero, "cast.fixed");
+CGF.Builder.CreateStore(Src, Dst, DstIsVolatile);
+return;
+  }
+}
+  }
   llvm::TypeSize DstSize = CGF.CGM.getDataLayout().getTypeAllocSize(DstTy);
 
   // If store is legal, just bitcast the src pointer.


Index: clang/test/CodeGen/attr-riscv-rvv-vector-bits-call.c
===
--- clang/test/CodeGen/attr-riscv-rvv-vector-bits-call.c
+++ clang/test/CodeGen/attr-riscv-rvv-vector-bits-call.c
@@ -38,11 +38,7 @@
 
 // CHECK-LABEL: @sizeless_caller(
 // CHECK-NEXT:  entry:
-// CHECK-NEXT:[[COERCE1:%.*]] = alloca <8 x i32>, align 8
-// CHECK-NEXT:store  [[X:%.*]], ptr [[COERCE1]], align 8
-// CHECK-NEXT:[[TMP0:%.*]] = load <8 x i32>, ptr [[COERCE1]], align 8, !tbaa [[TBAA4:![0-9]+]]
-// CHECK-NEXT:[[CASTSCALABLESVE2:%.*]] = tail call  @llvm.vector.insert.nxv2i32.v8i32( undef, <8 x i32> [[TMP0]], i64 0)
-// CHECK-NEXT:ret  [[CASTSCALABLESVE2]]
+// CHECK-NEXT:ret  [[X:%.*]]
 //
 vint32m1_t sizeless_caller(vint32m1_t x) {
   return fixed_callee(x);
Index: clang/test/CodeGen/attr-arm-sve-vector-bits-call.c
===
--- clang/test/CodeGen/attr-arm-sve-vector-bits-call.c
+++ clang/test/CodeGen/attr-arm-sve-vector-bits-call.c
@@ -41,11 +41,7 @@
 
 // CHECK-LABEL: @sizeless_caller(
 // 

[PATCH] D154869: [Flang] [FlangRT] Implement FlangRT library as solution to Flang's runtime LLVM integration

[Paul Scoropan via Phabricator via cfe-commits]

pscoro updated this revision to Diff 540110.
pscoro added a comment.

Fix build failures


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154869

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  flang-rt/CMakeLists.txt
  flang-rt/src/dummy.cpp
  flang/CMakeLists.txt
  flang/cmake/modules/AddFlang.cmake
  flang/lib/Decimal/CMakeLists.txt
  flang/runtime/CMakeLists.txt
  flang/test/CMakeLists.txt
  flang/test/Driver/linker-flags.f90
  flang/test/lit.cfg.py
  flang/tools/flang-driver/CMakeLists.txt
  lld/COFF/MinGW.cpp
  llvm/CMakeLists.txt
  llvm/projects/CMakeLists.txt
  runtimes/CMakeLists.txt

Index: runtimes/CMakeLists.txt
===
--- runtimes/CMakeLists.txt
+++ runtimes/CMakeLists.txt
@@ -19,7 +19,7 @@
 
 # We order libraries to mirror roughly how they are layered, except that compiler-rt can depend
 # on libc++, so we put it after.
-set(LLVM_DEFAULT_RUNTIMES "libc;libunwind;libcxxabi;pstl;libcxx;compiler-rt;openmp")
+set(LLVM_DEFAULT_RUNTIMES "libc;libunwind;libcxxabi;pstl;libcxx;compiler-rt;openmp;flang-rt")
 set(LLVM_SUPPORTED_RUNTIMES "${LLVM_DEFAULT_RUNTIMES};llvm-libgcc")
 set(LLVM_ENABLE_RUNTIMES "" CACHE STRING
   "Semicolon-separated list of runtimes to build, or \"all\" (${LLVM_DEFAULT_RUNTIMES}). Supported runtimes are ${LLVM_SUPPORTED_RUNTIMES}.")
Index: llvm/projects/CMakeLists.txt
===
--- llvm/projects/CMakeLists.txt
+++ llvm/projects/CMakeLists.txt
@@ -6,6 +6,7 @@
   if(IS_DIRECTORY ${entry} AND EXISTS ${entry}/CMakeLists.txt)
 if((NOT ${entry} STREQUAL ${CMAKE_CURRENT_SOURCE_DIR}/compiler-rt) AND
(NOT ${entry} STREQUAL ${CMAKE_CURRENT_SOURCE_DIR}/dragonegg) AND
+   (NOT ${entry} STREQUAL ${CMAKE_CURRENT_SOURCE_DIR}/flang-rt) AND
(NOT ${entry} STREQUAL ${CMAKE_CURRENT_SOURCE_DIR}/libcxx) AND
(NOT ${entry} STREQUAL ${CMAKE_CURRENT_SOURCE_DIR}/libcxxabi) AND
(NOT ${entry} STREQUAL ${CMAKE_CURRENT_SOURCE_DIR}/libunwind) AND
@@ -42,6 +43,8 @@
 add_llvm_external_project(dragonegg)
 add_llvm_external_project(openmp)
 
+add_llvm_external_project(flang-rt)
+
 if(LLVM_INCLUDE_TESTS)
   add_llvm_external_project(cross-project-tests)
 endif()
Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -149,7 +149,10 @@
 # As we migrate runtimes to using the bootstrapping build, the set of default runtimes
 # should grow as we remove those runtimes from LLVM_ENABLE_PROJECTS above.
 set(LLVM_DEFAULT_RUNTIMES "libcxx;libcxxabi;libunwind")
-set(LLVM_SUPPORTED_RUNTIMES "libc;libunwind;libcxxabi;pstl;libcxx;compiler-rt;openmp;llvm-libgcc")
+if ("flang" IN_LIST LLVM_ENABLE_PROJECTS)
+  set(LLVM_DEFAULT_RUNTIMES "libcxx;libcxxabi;libunwind;flang-rt")
+endif()
+set(LLVM_SUPPORTED_RUNTIMES "libc;libunwind;libcxxabi;pstl;libcxx;compiler-rt;openmp;llvm-libgcc;flang-rt")
 set(LLVM_ENABLE_RUNTIMES "" CACHE STRING
   "Semicolon-separated list of runtimes to build, or \"all\" (${LLVM_DEFAULT_RUNTIMES}). Supported runtimes are ${LLVM_SUPPORTED_RUNTIMES}.")
 if(LLVM_ENABLE_RUNTIMES STREQUAL "all")
@@ -171,6 +174,11 @@
   endif()
 endif()
 
+if ("flang" IN_LIST LLVM_ENABLE_PROJECTS AND NOT "flang-rt" IN_LIST LLVM_ENABLE_RUNTIMES)
+  message(STATUS "Enabling flang-rt to be built with the flang project")
+  list(APPEND LLVM_ENABLE_RUNTIMES "flang-rt")
+endif()
+
 # LLVM_ENABLE_PROJECTS_USED is `ON` if the user has ever used the
 # `LLVM_ENABLE_PROJECTS` CMake cache variable.  This exists for
 # several reasons:
Index: lld/COFF/MinGW.cpp
===
--- lld/COFF/MinGW.cpp
+++ lld/COFF/MinGW.cpp
@@ -50,8 +50,7 @@
   "libc++",
   "libc++abi",
   "libFortran_main",
-  "libFortranRuntime",
-  "libFortranDecimal",
+  "libflang-rt",
   "libunwind",
   "libmsvcrt",
   "libucrtbase",
Index: flang/tools/flang-driver/CMakeLists.txt
===
--- flang/tools/flang-driver/CMakeLists.txt
+++ flang/tools/flang-driver/CMakeLists.txt
@@ -19,8 +19,7 @@
   # These libraries are used in the linker invocation generated by the driver
   # (i.e. when constructing the linker job). Without them the driver would be
   # unable to generate executables.
-  FortranRuntime
-  FortranDecimal
+  flang-rt
   Fortran_main
 )
 
Index: flang/test/lit.cfg.py
===
--- flang/test/lit.cfg.py
+++ flang/test/lit.cfg.py
@@ -153,19 +153,16 @@
 # the C++ runtime libraries. For this we need a C compiler. If for some reason
 # we don't have one, we can just disable the test.
 if config.cc:
-libruntime = os.path.join(config.flang_lib_dir, "libFortranRuntime.a")
-libdecimal = os.path.join(config.flang_lib_dir, 

[PATCH] D154688: [clang] Show verify prefix in error messages

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D154688#4497967 , @tbaeder wrote:

> When passing a different prefix via `-verify=foo`, the error messages now say 
> "error: 'foo-error' diagnostics seen but not expected", etc.
>
> I'm often working in test files where two different prefixes are used and I'm 
> always confused about which one of the two the error messages are talking 
> about.

What I'm confused by is that we list the line numbers of the failures, so the 
prefix only seems like it's helpful in a case where two prefixes use the same 
message and the same severity on the same line. e.g., `// foo-error 
{{whatever}} bar-error {{whatever}}`. In the other cases, either the line 
number is different, or the severity is different, or the message is different 
which I thought was giving sufficient context.


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

https://reviews.llvm.org/D154688

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

[clang] d212e99 - [RISCV] Update test after the addition for rounding mode to vfadd intrinsic. NFC

[Craig Topper via cfe-commits]

Author: Craig Topper
Date: 2023-07-13T10:45:31-07:00
New Revision: d212e99bc5144696f95f09a573da6ed0d85601a0

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

LOG: [RISCV] Update test after the addition for rounding mode to vfadd 
intrinsic. NFC

The greediness of the operand matching regular expressions made
the test pass even though an operand is missing.

Added: 


Modified: 
clang/test/CodeGen/attr-riscv-rvv-vector-bits-call.c

Removed: 




diff  --git a/clang/test/CodeGen/attr-riscv-rvv-vector-bits-call.c 
b/clang/test/CodeGen/attr-riscv-rvv-vector-bits-call.c
index c9e0eeedbf26d6..330e4cd1124e4b 100644
--- a/clang/test/CodeGen/attr-riscv-rvv-vector-bits-call.c
+++ b/clang/test/CodeGen/attr-riscv-rvv-vector-bits-call.c
@@ -63,7 +63,7 @@ fixed_int32m1_t call_int32_ff(fixed_int32m1_t op1, 
fixed_int32m1_t op2) {
 
 // CHECK-LABEL: @call_float64_ff(
 // CHECK-NEXT:  entry:
-// CHECK-NEXT:[[TMP0:%.*]] = tail call  
@llvm.riscv.vfadd.nxv1f64.nxv1f64.i64( poison,  [[OP1_COERCE:%.*]],  [[OP2_COERCE:%.*]], i64 4)
+// CHECK-NEXT:[[TMP0:%.*]] = tail call  
@llvm.riscv.vfadd.nxv1f64.nxv1f64.i64( poison,  [[OP1_COERCE:%.*]],  [[OP2_COERCE:%.*]], i64 7, 
i64 4)
 // CHECK-NEXT:ret  [[TMP0]]
 //
 fixed_float64m1_t call_float64_ff(fixed_float64m1_t op1, fixed_float64m1_t 
op2) {
@@ -85,7 +85,7 @@ fixed_int32m1_t call_int32_fs(fixed_int32m1_t op1, vint32m1_t 
op2) {
 
 // CHECK-LABEL: @call_float64_fs(
 // CHECK-NEXT:  entry:
-// CHECK-NEXT:[[TMP0:%.*]] = tail call  
@llvm.riscv.vfadd.nxv1f64.nxv1f64.i64( poison,  [[OP1_COERCE:%.*]],  [[OP2:%.*]], i64 4)
+// CHECK-NEXT:[[TMP0:%.*]] = tail call  
@llvm.riscv.vfadd.nxv1f64.nxv1f64.i64( poison,  [[OP1_COERCE:%.*]],  [[OP2:%.*]], i64 7, i64 4)
 // CHECK-NEXT:ret  [[TMP0]]
 //
 fixed_float64m1_t call_float64_fs(fixed_float64m1_t op1, vfloat64m1_t op2) {
@@ -107,7 +107,7 @@ fixed_int32m1_t call_int32_ss(vint32m1_t op1, vint32m1_t 
op2) {
 
 // CHECK-LABEL: @call_float64_ss(
 // CHECK-NEXT:  entry:
-// CHECK-NEXT:[[TMP0:%.*]] = tail call  
@llvm.riscv.vfadd.nxv1f64.nxv1f64.i64( poison,  [[OP1:%.*]],  [[OP2:%.*]], i64 4)
+// CHECK-NEXT:[[TMP0:%.*]] = tail call  
@llvm.riscv.vfadd.nxv1f64.nxv1f64.i64( poison,  [[OP1:%.*]],  [[OP2:%.*]], i64 7, i64 4)
 // CHECK-NEXT:ret  [[TMP0]]
 //
 fixed_float64m1_t call_float64_ss(vfloat64m1_t op1, vfloat64m1_t op2) {



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

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86-64 datalayout

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> @efriedma would you consider the changes suggested by @hvdijk sufficient 
> under any circumstances or would you still insist on fully compatible 
> AutoUpgrade, given the above discussion?

If the requirement is "we can mix old and new IR", we have to do it correctly, 
to the extent old versions of clang do it correctly.

If we're willing to refuse to compile old IR and/or refuse to LTO together old 
and new IR, there are other possible solutions.  I'm not sure what workflows 
depend on having working autoupgrade.


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

https://reviews.llvm.org/D86310

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

[clang] c0de76c - [Driver] Remove unneeded useRelaxRelocations overrides

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2023-07-13T10:43:01-07:00
New Revision: c0de76cf9f06ba8356c212420d156d42ad1d15b8

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

LOG: [Driver] Remove unneeded useRelaxRelocations overrides

ENABLE_X86_RELAX_RELOCATIONS has defaulted to on
(c41a18cf61790fc898dcda1055c3efbf442c14c0) for nearly 3 years.
As a clean-up, remove overrides from some early adopters.

Change OHOS to use true as agreed by the patch author D145227.

Added: 


Modified: 
clang/lib/Driver/ToolChains/Fuchsia.h
clang/lib/Driver/ToolChains/OHOS.h
clang/lib/Driver/ToolChains/PS4CPU.h

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Fuchsia.h 
b/clang/lib/Driver/ToolChains/Fuchsia.h
index ba0ec208fb1299..95e1785c9fac97 100644
--- a/clang/lib/Driver/ToolChains/Fuchsia.h
+++ b/clang/lib/Driver/ToolChains/Fuchsia.h
@@ -56,7 +56,6 @@ class LLVM_LIBRARY_VISIBILITY Fuchsia : public ToolChain {
 
   bool HasNativeLLVMSupport() const override { return true; }
   bool IsMathErrnoDefault() const override { return false; }
-  bool useRelaxRelocations() const override { return true; };
   RuntimeLibType GetDefaultRuntimeLibType() const override {
 return ToolChain::RLT_CompilerRT;
   }

diff  --git a/clang/lib/Driver/ToolChains/OHOS.h 
b/clang/lib/Driver/ToolChains/OHOS.h
index a06df0eee6e2b1..2a380420922deb 100644
--- a/clang/lib/Driver/ToolChains/OHOS.h
+++ b/clang/lib/Driver/ToolChains/OHOS.h
@@ -36,7 +36,6 @@ class LLVM_LIBRARY_VISIBILITY OHOS : public Generic_ELF {
   bool isPICDefault() const override { return false; }
   bool isPIEDefault(const llvm::opt::ArgList ) const override { return 
true; }
   bool isPICDefaultForced() const override { return false; }
-  bool useRelaxRelocations() const override { return false; }
   UnwindLibType GetUnwindLibType(const llvm::opt::ArgList ) const 
override;
   UnwindLibType GetDefaultUnwindLibType() const override { return 
UNW_CompilerRT; }
 

diff  --git a/clang/lib/Driver/ToolChains/PS4CPU.h 
b/clang/lib/Driver/ToolChains/PS4CPU.h
index 0866a5daa4cc6c..a51351d367be84 100644
--- a/clang/lib/Driver/ToolChains/PS4CPU.h
+++ b/clang/lib/Driver/ToolChains/PS4CPU.h
@@ -101,8 +101,6 @@ class LLVM_LIBRARY_VISIBILITY PS4PS5Base : public 
Generic_ELF {
 return llvm::DenormalMode::getPreserveSign();
   }
 
-  bool useRelaxRelocations() const override { return true; }
-
   // Helper methods for PS4/PS5.
   virtual const char *getLinkerBaseName() const = 0;
   virtual std::string qualifyPSCmdName(StringRef CmdName) const = 0;



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

[PATCH] D153156: [Clang] CWG1473: do not err on the lack of space after operator""

[PoYao Chang via Phabricator via cfe-commits]

rZhBoYao updated this revision to Diff 540104.
rZhBoYao marked an inline comment as done.
rZhBoYao edited the summary of this revision.

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

https://reviews.llvm.org/D153156

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Lex/Lexer.cpp
  clang/test/CXX/drs/dr14xx.cpp
  clang/test/CXX/drs/dr17xx.cpp
  clang/test/CXX/drs/dr25xx.cpp
  clang/test/CXX/lex/lex.literal/lex.ext/p1.cpp
  clang/test/CXX/lex/lex.literal/lex.ext/p10.cpp
  clang/test/CXX/lex/lex.literal/lex.ext/p11.cpp
  clang/test/CXX/lex/lex.literal/lex.ext/p3.cpp
  clang/test/CXX/lex/lex.literal/lex.ext/p4.cpp
  clang/test/CXX/lex/lex.literal/lex.ext/p5.cpp
  clang/test/CXX/lex/lex.literal/lex.ext/p6.cpp
  clang/test/CXX/lex/lex.literal/lex.ext/p7.cpp
  clang/test/CXX/lex/lex.literal/lex.ext/p8.cpp
  clang/test/CXX/lex/lex.literal/lex.ext/p9.cpp
  clang/test/CXX/over/over.oper/over.literal/p2.cpp
  clang/test/CXX/over/over.oper/over.literal/p3.cpp
  clang/test/CXX/over/over.oper/over.literal/p5.cpp
  clang/test/CXX/over/over.oper/over.literal/p6.cpp
  clang/test/CXX/over/over.oper/over.literal/p7.cpp
  clang/test/CXX/over/over.oper/over.literal/p8.cpp
  clang/test/CodeGenCXX/cxx11-user-defined-literal.cpp
  clang/test/CodeGenCXX/mangle-ms-cxx11.cpp
  clang/test/FixIt/fixit-c++11.cpp
  clang/test/OpenMP/amdgcn_ldbl_check.cpp
  clang/test/PCH/cxx11-user-defined-literals.cpp
  clang/test/Parser/cxx0x-literal-operators.cpp
  clang/test/Parser/cxx11-user-defined-literals.cpp
  clang/test/SemaCXX/cxx11-ast-print.cpp
  clang/test/SemaCXX/cxx11-user-defined-literals-unused.cpp
  clang/test/SemaCXX/cxx11-user-defined-literals.cpp
  clang/test/SemaCXX/cxx1y-user-defined-literals.cpp
  clang/test/SemaCXX/cxx1z-user-defined-literals.cpp
  clang/test/SemaCXX/cxx2a-consteval.cpp
  clang/test/SemaCXX/cxx98-compat.cpp
  clang/test/SemaCXX/literal-operators.cpp
  clang/test/SemaCXX/no-warn-user-defined-literals-in-system-headers.cpp
  clang/test/SemaCXX/reserved-identifier.cpp
  clang/test/SemaCXX/warn-xor-as-pow.cpp
  clang/www/cxx_dr_status.html

Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -8645,7 +8645,7 @@
 https://cplusplus.github.io/CWG/issues/1473.html;>1473
 CD3
 Syntax of literal-operator-id
-Unknown
+Clang 17
   
   
 https://cplusplus.github.io/CWG/issues/1474.html;>1474
Index: clang/test/SemaCXX/warn-xor-as-pow.cpp
===
--- clang/test/SemaCXX/warn-xor-as-pow.cpp
+++ clang/test/SemaCXX/warn-xor-as-pow.cpp
@@ -19,10 +19,10 @@
 #define flexor 7
 
 #ifdef __cplusplus
-constexpr long long operator"" _xor(unsigned long long v) { return v; }
+constexpr long long operator""_xor(unsigned long long v) { return v; }
 
-constexpr long long operator"" _0b(unsigned long long v) { return v; }
-constexpr long long operator"" _0X(unsigned long long v) { return v; }
+constexpr long long operator""_0b(unsigned long long v) { return v; }
+constexpr long long operator""_0X(unsigned long long v) { return v; }
 #else
 #define xor ^ // iso646.h
 #endif
Index: clang/test/SemaCXX/reserved-identifier.cpp
===
--- clang/test/SemaCXX/reserved-identifier.cpp
+++ clang/test/SemaCXX/reserved-identifier.cpp
@@ -89,7 +89,7 @@
 long double sacrebleu = operator"" _SacreBleu(1.2); // expected-warning {{identifier '_SacreBleu' is reserved because it starts with '_' followed by a capital letter}}
 long double sangbleu = operator""_SacreBleu(1.2);   // no-warning
 
-void operator"" _lowercase(unsigned long long); // no-warning
+void operator""_lowercase(unsigned long long); // no-warning
 void operator""_lowercase(unsigned long long); // no-warning
 
 struct _BarbeRouge { // expected-warning {{identifier '_BarbeRouge' is reserved because it starts with '_' followed by a capital letter}}
Index: clang/test/SemaCXX/no-warn-user-defined-literals-in-system-headers.cpp
===
--- clang/test/SemaCXX/no-warn-user-defined-literals-in-system-headers.cpp
+++ clang/test/SemaCXX/no-warn-user-defined-literals-in-system-headers.cpp
@@ -2,4 +2,4 @@
 
 #include 
 
-void operator "" bar(long double); // expected-warning{{user-defined literal suffixes not starting with '_' are reserved}}
+void operator ""bar(long double); // expected-warning{{user-defined literal suffixes not starting with '_' are reserved}}
Index: clang/test/SemaCXX/literal-operators.cpp
===
--- clang/test/SemaCXX/literal-operators.cpp
+++ clang/test/SemaCXX/literal-operators.cpp
@@ -3,46 +3,46 @@
 #include 
 
 struct tag {
-  void operator "" _tag_bad (const char *); // 

[PATCH] D152632: [Clang] Add warnings for CWG2521

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM assuming precommit CI doesn't find any surprises.


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

https://reviews.llvm.org/D152632

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

[PATCH] D4784: [clang-tidy] Add check for possibly incomplete switch statements

[Shivam Gupta via Phabricator via cfe-commits]

xgupta updated this revision to Diff 540102.
xgupta added a comment.

Updated SwitchMissingDefaultCaseCheck.cpp but still WIP
Will look again on saturday/sunday


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D4784

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SwitchMissingDefaultCaseCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SwitchMissingDefaultCaseCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone/switch-missing-default-case.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/switch-missing-default-case.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/switch-missing-default-case.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/switch-missing-default-case.cpp
@@ -0,0 +1,66 @@
+// RUN: %check_clang_tidy %s bugprone-switch-missing-default-case -- %t
+
+typedef int MyInt;
+typedef enum { eE2 } MyEnum;
+
+void positive() {
+  int I1 = 0;
+  // CHECK-MESSAGES: [[@LINE+1]]:11: warning: switching on non-enum value without default case may not cover all cases [bugprone-switch-missing-default-case]
+  switch (I1) {
+  case 0:
+break;
+  }
+
+  MyInt I2 = 0;
+  // CHECK-MESSAGES: [[@LINE+1]]:11: warning: switching on non-enum value without default case may not cover all cases [bugprone-switch-missing-default-case]
+  switch (I2) {
+  case 0:
+break;
+  }
+
+  int getValue();
+  // CHECK-MESSAGES: [[@LINE+1]]:11: warning: switching on non-enum value without default case may not cover all cases [bugprone-switch-missing-default-case]
+  switch (getValue()) {
+  case 0:
+break;
+  }
+}
+
+void negative() {
+  enum E { eE1 };
+  E E1 = eE1;
+  switch (E1) { // no-warning
+  case eE1:
+break;
+  }
+
+  MyEnum E2 = eE2;
+  switch (E2) { // no-warning
+  case eE2:
+break;
+  }
+
+  int I1 = 0;
+  switch (I1) { // no-warning
+  case 0:
+break;
+  default:
+break;
+  }
+
+  MyInt I2 = 0;
+  switch (I2) { // no-warning
+  case 0:
+break;
+  default:
+break;
+  }
+
+  int getValue();
+  switch (getValue()) { // no-warning
+  case 0:
+break;
+default:
+break;
+  }
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -92,6 +92,7 @@
`bugprone-forwarding-reference-overload `_,
`bugprone-implicit-widening-of-multiplication-result `_, "Yes"
`bugprone-inaccurate-erase `_, "Yes"
+   `bugprone-switch-missing-default-case `_, "Yes"
`bugprone-incorrect-roundings `_,
`bugprone-infinite-loop `_,
`bugprone-integer-division `_,
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/switch-missing-default-case.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/switch-missing-default-case.rst
@@ -0,0 +1,61 @@
+.. title:: clang-tidy - bugprone-switch-missing-default-case
+
+bugprone-switch-missing-default-case
+
+
+Ensures that switch statements without default cases are flagged, focuses only
+on covering cases with non-enums where the compiler may not issue warnings.
+
+Switch statements without a default case can lead to unexpected
+behavior and incomplete handling of all possible cases. When a switch statement
+lacks a default case, if a value is encountered that does not match any of the
+specified cases, the program will continue execution without any defined
+behavior or handling.
+
+This check helps identify switch statements that are missing a default case,
+allowing developers to ensure that all possible cases are handled properly.
+Adding a default case allows for graceful handling of unexpected or unmatched
+values, reducing the risk of program errors and unexpected behavior.
+
+Example:
+
+.. code-block:: c++
+
+  // Example 1:
+  // warning: switching on non-enum value without default case may not cover all cases
+  switch (i) {
+  case 0:
+break;
+  }
+
+  // Example 2:
+  enum E { eE1 };
+  E e = eE1;
+  switch (e) { // no-warning
+  case eE1:
+break;
+  }
+
+  // Example 3:
+  int i = 0;
+  switch (i) { // no-warning
+  case 0:
+break;
+  default:
+break;
+  }
+
+.. note::
+   Enum types are already covered by compiler warnings when a switch statement
+   does not handle all enum values. This check focuses on non-enum types where
+   the compiler warnings may not be present. Some common examples of non-enum
+   types include:
+   - Integral types (int, char, short, etc.)
+   - Floating-point types (float, double)

[PATCH] D4784: [clang-tidy] Add check for possibly incomplete switch statements

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone/switch-missing-default-case.rst:52
+.. note::
+   Enum types are already covered by compiler warnings when a switch statement
+   does not handle all enum values. This check focuses on non-enum types where

xgupta wrote:
> PiotrZSL wrote:
> > would be nice to list them
> Didn't understand, what should I list, non-enum types?
No, not a types, compiler warnings (ones for enums). Simply so when user enable 
this check for integers, He/she could make sure that enum specific warnings are 
also enabled.
and remove this list that you added, as you cannot do switch on pointers or 
floating point types.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone/switch-missing-default-case.cpp:4
+typedef int MyInt;
+typedef enum { eE2 } MyEnum;
+




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D4784

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

[PATCH] D155220: [IRGen] Remove 'Sve' from the name of some IR names that are shared with RISC-V now.

[Craig Topper via Phabricator via cfe-commits]

craig.topper created this revision.
craig.topper added reviewers: joechrisellis, bsmith, c-rhodes, sdesmalen.
Herald added subscribers: VincentWu, vkmr, luismarques, sameer.abuasal, 
s.egerton, Jim, rogfer01, shiva0217, kito-cheng, simoncook, arichardson.
Herald added a project: All.
craig.topper requested review of this revision.
Herald added a subscriber: wangpc.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155220

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGExprScalar.cpp


Index: clang/lib/CodeGen/CGExprScalar.cpp
===
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -2152,7 +2152,7 @@
   llvm::Value *UndefVec = llvm::UndefValue::get(DstTy);
   llvm::Value *Zero = llvm::Constant::getNullValue(CGF.CGM.Int64Ty);
   llvm::Value *Result = Builder.CreateInsertVector(
-  DstTy, UndefVec, Src, Zero, "castScalableSve");
+  DstTy, UndefVec, Src, Zero, "cast.scalable");
   if (NeedsBitCast)
 Result = Builder.CreateBitCast(Result, OrigType);
   return Result;
@@ -2176,7 +2176,7 @@
 }
 if (ScalableSrc->getElementType() == FixedDst->getElementType()) {
   llvm::Value *Zero = llvm::Constant::getNullValue(CGF.CGM.Int64Ty);
-  return Builder.CreateExtractVector(DstTy, Src, Zero, "castFixedSve");
+  return Builder.CreateExtractVector(DstTy, Src, Zero, "cast.fixed");
 }
   }
 }
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1311,7 +1311,7 @@
 auto *UndefVec = llvm::UndefValue::get(ScalableDst);
 auto *Zero = llvm::Constant::getNullValue(CGF.CGM.Int64Ty);
 llvm::Value *Result = CGF.Builder.CreateInsertVector(
-ScalableDst, UndefVec, Load, Zero, "castScalableSve");
+ScalableDst, UndefVec, Load, Zero, "cast.scalable");
 if (NeedsBitcast)
   Result = CGF.Builder.CreateBitCast(Result, OrigType);
 return Result;
@@ -3146,7 +3146,7 @@
 assert(NumIRArgs == 1);
 Coerced->setName(Arg->getName() + ".coerce");
 
ArgVals.push_back(ParamValue::forDirect(Builder.CreateExtractVector(
-VecTyTo, Coerced, Zero, "castFixedSve")));
+VecTyTo, Coerced, Zero, "cast.fixed")));
 break;
   }
 }


Index: clang/lib/CodeGen/CGExprScalar.cpp
===
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -2152,7 +2152,7 @@
   llvm::Value *UndefVec = llvm::UndefValue::get(DstTy);
   llvm::Value *Zero = llvm::Constant::getNullValue(CGF.CGM.Int64Ty);
   llvm::Value *Result = Builder.CreateInsertVector(
-  DstTy, UndefVec, Src, Zero, "castScalableSve");
+  DstTy, UndefVec, Src, Zero, "cast.scalable");
   if (NeedsBitCast)
 Result = Builder.CreateBitCast(Result, OrigType);
   return Result;
@@ -2176,7 +2176,7 @@
 }
 if (ScalableSrc->getElementType() == FixedDst->getElementType()) {
   llvm::Value *Zero = llvm::Constant::getNullValue(CGF.CGM.Int64Ty);
-  return Builder.CreateExtractVector(DstTy, Src, Zero, "castFixedSve");
+  return Builder.CreateExtractVector(DstTy, Src, Zero, "cast.fixed");
 }
   }
 }
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1311,7 +1311,7 @@
 auto *UndefVec = llvm::UndefValue::get(ScalableDst);
 auto *Zero = llvm::Constant::getNullValue(CGF.CGM.Int64Ty);
 llvm::Value *Result = CGF.Builder.CreateInsertVector(
-ScalableDst, UndefVec, Load, Zero, "castScalableSve");
+ScalableDst, UndefVec, Load, Zero, "cast.scalable");
 if (NeedsBitcast)
   Result = CGF.Builder.CreateBitCast(Result, OrigType);
 return Result;
@@ -3146,7 +3146,7 @@
 assert(NumIRArgs == 1);
 Coerced->setName(Arg->getName() + ".coerce");
 ArgVals.push_back(ParamValue::forDirect(Builder.CreateExtractVector(
-VecTyTo, Coerced, Zero, "castFixedSve")));
+VecTyTo, Coerced, Zero, "cast.fixed")));
 break;
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149867: [Clang][M68k] Add Clang support for the new M68k_RTD CC

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Also, it looks like precommit CI found a relevant failure that should be 
investigated.


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

https://reviews.llvm.org/D149867

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

[PATCH] D149867: [Clang][M68k] Add Clang support for the new M68k_RTD CC

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Sorry for the long delay in review on this, it slipped through the cracks for 
me.




Comment at: clang/lib/AST/TypePrinter.cpp:1861
 break;
+  case attr::M68kRTD: OS << "m68k_rtd"; break;
   case attr::NoDeref:





Comment at: clang/test/Sema/m68k-mrtd.c:4-9
+#ifdef MRTD
+// expected-error@+3 {{function with no prototype cannot use the m68k_rtd 
calling convention}}
+#endif
+void foo(int arg) {
+  bar(arg);
+}

myhsu wrote:
> aaron.ballman wrote:
> > A better way to do this is to use `-verify=mrtd` on the line enabling rtd, 
> > and using `// rtd-error {{whatever}}` on the line being diagnosed. (Same 
> > comment applies throughout the file.)
> > 
> > Huh, I was unaware that implicit function declarations are using something 
> > other than the default calling convention (which is C, not m68k_rtd). Is 
> > this intentional?
> > Huh, I was unaware that implicit function declarations are using something 
> > other than the default calling convention (which is C, not m68k_rtd). Is 
> > this intentional?
> 
> I'm not sure if I understand you correctly, but this diagnostic is emitted if 
> the CC does not support variadic function call. 
`bar` is not declared, in C89 this causes an implicit function declaration to 
be generated with the signature: `extern int ident();` What surprised me is 
that we seem to be generating something more like this instead: 
`__attribute__((m68k_rtd)) int ident();` despite that not being valid.



Comment at: clang/test/Sema/m68k-mrtd.c:45
+extern void (*d)(int, ...);
+__attribute__((m68k_rtd)) extern void (*d)(int, ...);

myhsu wrote:
> aaron.ballman wrote:
> > Missing tests for:
> > 
> > * Function without a prototype
> > * Applying the attribute to a non-function
> > * Providing arguments to the attribute
> > * What should happen for C++ and things like member functions?
> > Function without a prototype
> 
> I thought the first check was testing function without a prototype.
> 
> > What should happen for C++ and things like member functions?
> 
> I believe we don't have any special handling for C++.
> 
> I addressed rest of the bullet items you mentioned, please take a look.
>> Function without a prototype
> I thought the first check was testing function without a prototype.

Nope, I meant something more like this:
```
__attribute__((m68k_rtd)) void foo(); // Should error
__attribute__((m68k_rtd)) void bar(a, b) int a, b; {} // Should error
```

>> What should happen for C++ and things like member functions?
> I believe we don't have any special handling for C++.

Test coverage should still exist to ensure the behavior is what you'd expect 
when adding the calling convention to a member function and a lambda, for 
example. (Both Sema and CodeGen tests for this)

Also missing coverage for the changes to `-fdefault-calling-conv=`

I'm also still wondering whether there's a way to use m68k with a Windows ABI 
triple (basically, do we need changes in MicrosoftMangle.cpp?)


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

https://reviews.llvm.org/D149867

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

[PATCH] D155131: [clang][modules] Deserialize included files lazily

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

In D155131#4495489 , @benlangmuir 
wrote:

> Now that it's not eagerly deserialized, should 
> `Preprocessor::alreadyIncluded` call `HeaderInfo.getFileInfo(File)` to ensure 
> the information is up to date?

It should, but `Preprocessor::alreadyIncluded()` is only called from 
`HeaderSearch::ShouldEnterIncludeFile()` and 
`Preprocessor::HandleHeaderIncludeOrImport()`, where 
`HeaderSearch::getFileInfo(File)` has already been called. But I agree it would 
be better to ensure that within `Preprocessor::alreadyIncluded()` itself. I'll 
try to include that in the next revision.

> Similarly, we expose the list of files in `Preprocessor::getIncludedFiles` -- 
> is it okay if this list is incomplete?

That should be okay. This function only needs to return files included in the 
current module compilation, not all transitive includes.




Comment at: clang/lib/Serialization/ASTReader.cpp:1947
+if (const FileEntry *FE = getFile(key))
+  Reader.getPreprocessor().getIncludedFiles().insert(FE);
+

benlangmuir wrote:
> `Reader.getPreprocessor().markIncluded`?
That would trigger infinite recursion, since that calls `getFileInfo()` which 
attempts to deserialize.



Comment at: clang/lib/Serialization/ASTWriter.cpp:2545
-raw_svector_ostream Out(Buffer);
-writeIncludedFiles(Out, PP);
-RecordData::value_type Record[] = {PP_INCLUDED_FILES};

benlangmuir wrote:
> Can we remove `ASTWriter::writeIncludedFiles` now?
Yes, forgot about that, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155131

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

[PATCH] D152818: [Clang] Make template instantiations respect pragma FENV_ACCESS state at point-of-definition

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D152818#4457013 , @zahiraam wrote:

> In D152818#4456872 , @aaron.ballman 
> wrote:
>
>> In D152818#4456797 , @zahiraam 
>> wrote:
>>
>>> In D152818#4456717 , 
>>> @aaron.ballman wrote:
>>>
 In D152818#4456510 , @zahiraam 
 wrote:

> In D152818#4456483 , 
> @nicolerabjohn wrote:
>
>> In D152818#4442116 , @rjmccall 
>> wrote:
>>
>>> Does https://reviews.llvm.org/D143241 solve the original problem here, 
>>> or is there something deeper?
>>
>> It does not solve the problem, at least for my test case (linked in 
>> https://github.com/llvm/llvm-project/issues/63063) - we still hit the 
>> assertion.
>
> Sorry! I will not be able to work on this until about September! I 
> haven't tried to reproduce the issue with the test case in the link above.

 We branch for the 17 release at the end of July, so I'm wondering whether 
 there's anything we need to revert related to this? It looks like this 
 assertion started firing in Clang 12.0.0 
 (https://cexplorer.testlabs.pro/z/Wh9o8W), so this doesn't seem to be a 
 regression, but confirmation would be appreciated.
>>>
>>> The code that is generating the assertion has been introduced by this 
>>> patch: https://reviews.llvm.org/D80462 
>>> I can checkout that commit and see if the test case fails with it? Would 
>>> that be a good experiment to do?
>>
>> No need -- so long as we're all agreed that we haven't regressed anything 
>> between Clang 16 and Clang 17 here, that's all I'm really after.
>
> I verified that it's failing from clang12.0.0 through clang 16.0.0. I think 
> this confirms that's not a regression between clang 16 and clang 17. Doesn't 
> it?

Correct, it does, so nothing needs to be reverted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152818

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

[PATCH] D155213: [HIP] Add `-fno-hip-uniform-block`

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:7226
+  } else {
+Args.claimAllArgs(options::OPT_fhip_uniform_block,
+  options::OPT_fno_hip_uniform_block);

yaxunl wrote:
> MaskRay wrote:
> > Why is the -Wunused-command-line-argument warning suppressed in non-IsHIP 
> > mode?
> Users may want to add these options to clang config file.
> 
> Is there a general rule which options should be claimed?
Options in a configuration file are automatically claimed.

I don't know a general rule, but we generally don't claim newly introduced 
options.


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

https://reviews.llvm.org/D155213

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

[PATCH] D152206: [Basic] Support 64-bit x86 target for UEFI

[Prabhu Karthikeyan Rajasekaran via Phabricator via cfe-commits]

Prabhuk updated this revision to Diff 540091.
Prabhuk marked 5 inline comments as done.
Prabhuk added a comment.

Minor formatting changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152206

Files:
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Basic/Targets/X86.h
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/UEFI.cpp
  clang/lib/Driver/ToolChains/UEFI.h
  clang/test/Driver/uefi.c
  llvm/lib/TargetParser/Triple.cpp
  llvm/unittests/TargetParser/TripleTest.cpp

Index: llvm/unittests/TargetParser/TripleTest.cpp
===
--- llvm/unittests/TargetParser/TripleTest.cpp
+++ llvm/unittests/TargetParser/TripleTest.cpp
@@ -354,6 +354,7 @@
   EXPECT_EQ(Triple::UnknownVendor, T.getVendor());
   EXPECT_EQ(Triple::UEFI, T.getOS());
   EXPECT_EQ(Triple::UnknownEnvironment, T.getEnvironment());
+  EXPECT_EQ(Triple::COFF, T.getObjectFormat());
 
   T = Triple("wasm32-unknown-unknown");
   EXPECT_EQ(Triple::wasm32, T.getArch());
Index: llvm/lib/TargetParser/Triple.cpp
===
--- llvm/lib/TargetParser/Triple.cpp
+++ llvm/lib/TargetParser/Triple.cpp
@@ -804,6 +804,8 @@
   return Triple::MachO;
 else if (T.isOSWindows())
   return Triple::COFF;
+else if (T.isUEFI())
+  return Triple::COFF;
 return Triple::ELF;
 
   case Triple::aarch64_be:
Index: clang/test/Driver/uefi.c
===
--- /dev/null
+++ clang/test/Driver/uefi.c
@@ -0,0 +1,10 @@
+// RUN: %clang -### %s --target=x86_64-unknown-uefi \
+// RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 \
+// RUN: | FileCheck -check-prefixes=CHECK %s
+// RUN: %clang -### %s --target=x86_64-uefi \
+// RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 \
+// RUN: | FileCheck -check-prefixes=CHECK %s
+// CHECK: "-cc1"
+// CHECK-SAME: "-triple" "x86_64-unknown-uefi"
+// CHECK-SAME: "-mrelocation-model" "pic" "-pic-level" "2"
+// CHECK-SAME: "-mframe-pointer=all"
Index: clang/lib/Driver/ToolChains/UEFI.h
===
--- /dev/null
+++ clang/lib/Driver/ToolChains/UEFI.h
@@ -0,0 +1,61 @@
+//===--- UEFI.h - UEFI ToolChain Implementations --*- 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
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_UEFI_H
+#define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_UEFI_H
+
+#include "clang/Driver/Tool.h"
+#include "clang/Driver/ToolChain.h"
+
+namespace clang {
+namespace driver {
+namespace tools {
+namespace uefi {
+class LLVM_LIBRARY_VISIBILITY Linker : public Tool {
+public:
+  Linker(const ToolChain ) : Tool("uefi::Linker", "lld-link", TC) {}
+
+  bool hasIntegratedCPP() const override { return false; }
+  bool isLinkJob() const override { return true; }
+
+  void ConstructJob(Compilation , const JobAction ,
+const InputInfo , const InputInfoList ,
+const llvm::opt::ArgList ,
+const char *LinkingOutput) const override;
+};
+} // end namespace uefi
+} // end namespace tools
+
+namespace toolchains {
+
+class LLVM_LIBRARY_VISIBILITY UEFI : public ToolChain {
+public:
+  UEFI(const Driver , const llvm::Triple ,
+   const llvm::opt::ArgList );
+
+protected:
+  Tool *buildLinker() const override;
+
+public:
+  bool HasNativeLLVMSupport() const override { return true; }
+  bool IsIntegratedAssemblerDefault() const override { return true; }
+  bool IsUnwindTablesDefault(const llvm::opt::ArgList ) const {
+return true;
+  }
+  bool isPICDefault() const override { return true; }
+  bool isPIEDefault(const llvm::opt::ArgList ) const override {
+return false;
+  }
+  bool isPICDefaultForced() const override { return true; }
+};
+
+} // namespace toolchains
+} // namespace driver
+} // namespace clang
+
+#endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_UEFI_H
Index: clang/lib/Driver/ToolChains/UEFI.cpp
===
--- /dev/null
+++ clang/lib/Driver/ToolChains/UEFI.cpp
@@ -0,0 +1,109 @@
+//===--- UEFI.h - UEFI ToolChain Implementations --*- 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
+//
+//===--===//
+
+#include "UEFI.h"
+#include "CommonArgs.h"
+#include "Darwin.h"
+#include "clang/Basic/CharInfo.h"
+#include 

[PATCH] D152206: [Basic] Support 64-bit x86 target for UEFI

[Prabhu Karthikeyan Rajasekaran via Phabricator via cfe-commits]

Prabhuk updated this revision to Diff 540085.
Prabhuk marked 6 inline comments as done.
Prabhuk added a comment.

Added subsystem and entry args. Improved driver test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152206

Files:
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Basic/Targets/X86.h
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/UEFI.cpp
  clang/lib/Driver/ToolChains/UEFI.h
  clang/test/Driver/uefi.c
  llvm/lib/TargetParser/Triple.cpp
  llvm/unittests/TargetParser/TripleTest.cpp

Index: llvm/unittests/TargetParser/TripleTest.cpp
===
--- llvm/unittests/TargetParser/TripleTest.cpp
+++ llvm/unittests/TargetParser/TripleTest.cpp
@@ -354,6 +354,7 @@
   EXPECT_EQ(Triple::UnknownVendor, T.getVendor());
   EXPECT_EQ(Triple::UEFI, T.getOS());
   EXPECT_EQ(Triple::UnknownEnvironment, T.getEnvironment());
+  EXPECT_EQ(Triple::COFF, T.getObjectFormat());
 
   T = Triple("wasm32-unknown-unknown");
   EXPECT_EQ(Triple::wasm32, T.getArch());
Index: llvm/lib/TargetParser/Triple.cpp
===
--- llvm/lib/TargetParser/Triple.cpp
+++ llvm/lib/TargetParser/Triple.cpp
@@ -804,6 +804,8 @@
   return Triple::MachO;
 else if (T.isOSWindows())
   return Triple::COFF;
+else if (T.isUEFI())
+  return Triple::COFF;
 return Triple::ELF;
 
   case Triple::aarch64_be:
Index: clang/test/Driver/uefi.c
===
--- /dev/null
+++ clang/test/Driver/uefi.c
@@ -0,0 +1,10 @@
+// RUN: %clang -### %s --target=x86_64-unknown-uefi \
+// RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 \
+// RUN: | FileCheck -check-prefixes=CHECK %s
+// RUN: %clang -### %s --target=x86_64-uefi \
+// RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 \
+// RUN: | FileCheck -check-prefixes=CHECK %s
+// CHECK: "-cc1"
+// CHECK-SAME: "-triple" "x86_64-unknown-uefi"
+// CHECK-SAME: "-mrelocation-model" "pic" "-pic-level" "2"
+// CHECK-SAME: "-mframe-pointer=all"
\ No newline at end of file
Index: clang/lib/Driver/ToolChains/UEFI.h
===
--- /dev/null
+++ clang/lib/Driver/ToolChains/UEFI.h
@@ -0,0 +1,60 @@
+//===--- UEFI.h - UEFI ToolChain Implementations --*- 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
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_UEFI_H
+#define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_UEFI_H
+
+#include "clang/Driver/Tool.h"
+#include "clang/Driver/ToolChain.h"
+
+namespace clang {
+namespace driver {
+namespace tools {
+namespace uefi {
+class LLVM_LIBRARY_VISIBILITY Linker : public Tool {
+public:
+  Linker(const ToolChain ) : Tool("uefi::Linker", "lld-link", TC) {}
+
+  bool hasIntegratedCPP() const override { return false; }
+  bool isLinkJob() const override { return true; }
+
+  void ConstructJob(Compilation , const JobAction ,
+const InputInfo , const InputInfoList ,
+const llvm::opt::ArgList ,
+const char *LinkingOutput) const override;
+};
+} // end namespace uefi
+} // end namespace tools
+
+namespace toolchains {
+
+class LLVM_LIBRARY_VISIBILITY UEFI : public ToolChain {
+public:
+  UEFI(const Driver , const llvm::Triple ,
+   const llvm::opt::ArgList );
+
+protected:
+  Tool *buildLinker() const override;
+
+public:
+  bool HasNativeLLVMSupport() const override { return true; }
+  bool IsIntegratedAssemblerDefault() const override { return true; }
+  bool IsUnwindTablesDefault(const llvm::opt::ArgList ) const {
+return true;
+  }
+  bool isPICDefault() const override { return true; }
+  bool isPIEDefault(const llvm::opt::ArgList ) const override {
+return false;
+  }
+  bool isPICDefaultForced() const override { return true; }
+};
+
+} // namespace toolchains
+} // namespace driver
+} // namespace clang
+#endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_UEFI_H
Index: clang/lib/Driver/ToolChains/UEFI.cpp
===
--- /dev/null
+++ clang/lib/Driver/ToolChains/UEFI.cpp
@@ -0,0 +1,109 @@
+//===--- UEFI.h - UEFI ToolChain Implementations --*- 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
+//
+//===--===//
+
+#include "UEFI.h"
+#include "CommonArgs.h"
+#include "Darwin.h"

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86-64 datalayout

[Harald van Dijk via Phabricator via cfe-commits]

hvdijk added a comment.

In D86310#4496582 , @nikic wrote:

> The main problem with that is that we can't have multiple data layouts for 
> one module, so linking old and new bitcode together would fail.

Good point, but it's worth pointing out that this only applies to linking in 
the LLVM IR sense. Linking in the ELF object file sense would work exactly as 
it would with the explicit alignments added everywhere, as ELF object files do 
not contain that data layout string. Linking in the LLVM IR sense is what 
happens with `clang -flto` though.

> But maybe that's exactly what we want -- after all, it is incompatible. Even 
> if we "correctly" upgraded to preserve behavior of the old bitcode, it would 
> still be incompatible with the new bitcode if i128 crosses the ABI boundary 
> (explicitly or implicitly).

Yeah, that is a tricky question to answer. Let's say this change goes into LLVM 
17, so LLVM 17 X86 data layouts include `i128:128`, and nothing is changed for 
LLVM 16. Let's also say we have a program made up of two source files, `a.c`, 
and `b.c`. Then:

- `clang-16 -c -flto a.c b.c && clang-17 a.o b.o` should ideally be accepted 
and would behave in the same way as `clang-16 -c a.c b.c && clang-16 a.o b.o`.
- `clang-16 -c -flto a.c && clang-17 -c -flto b.c && clang-17 a.o b.o` should 
ideally be accepted if neither `a.o` nor `b.o` use


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

https://reviews.llvm.org/D86310

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

[PATCH] D153701: [WIP][Clang] Implement P2718R0 "Lifetime extension in range-based for loops"

[Yurong via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaExprCXX.cpp:8901-8914
+  // [P2718R0] Lifetime extension in range-based for loops.
+  //
+  // 6.7.7 [class.temporary] p5:
+  // There are four contexts in which temporaries are destroyed at a different
+  // point than the end of the full-expression.
+  //
+  // 6.7.7 [class.temporary] p6:

hubert.reinterpretcast wrote:
> yronglin wrote:
> > hubert.reinterpretcast wrote:
> > > yronglin wrote:
> > > > yronglin wrote:
> > > > > hubert.reinterpretcast wrote:
> > > > > > yronglin wrote:
> > > > > > > rsmith wrote:
> > > > > > > > This isn't the right way to model the behavior here -- the 
> > > > > > > > presence or absence of an `ExprWithCleanups` is just a 
> > > > > > > > convenience to tell consumers of the AST whether they should 
> > > > > > > > expect to see cleanups later or not, and doesn't carry an 
> > > > > > > > implication of affecting the actual temporary lifetimes and 
> > > > > > > > storage durations.
> > > > > > > > 
> > > > > > > > The outcome that we should be aiming to reach is that all 
> > > > > > > > `MaterializeTemporaryExpr`s created as part of processing the 
> > > > > > > > for-range-initializer are marked as being lifetime-extended by 
> > > > > > > > the for-range variable. Probably the simplest way to handle 
> > > > > > > > that would be to track the current enclosing 
> > > > > > > > for-range-initializer variable in the 
> > > > > > > > `ExpressionEvaluationContextRecord`, and whenever a 
> > > > > > > > `MaterializeTemporaryExpr` is created, if there is a current 
> > > > > > > > enclosing for-range-initializer, mark that 
> > > > > > > > `MaterializeTemporaryExpr` as being lifetime-extended by it.
> > > > > > > Awesome! Thanks a lot for your advice, this is very helpful! I 
> > > > > > > want to take a longer look at it.
> > > > > > As mentioned in D139586, `CXXDefaultArgExpr`s may need additional 
> > > > > > handling. Similarly for `CXXDefaultInitExpr`s.
> > > > > Thanks for your tips! I have a question that what's the correct way 
> > > > > to extent the lifetime of `CXXBindTemporaryExpr`? Can I just 
> > > > > `materialize` the temporary? It may replaced by 
> > > > > `MaterializeTemporaryExpr`, and then I can mark it as being 
> > > > > lifetime-extended by the for-range variable.
> > > > Eg.
> > > > ```
> > > > void f() {
> > > >   int v[] = {42, 17, 13};
> > > >   Mutex m;
> > > >   for (int x : static_cast(LockGuard(m)), v) // lock released in 
> > > > C++ 2020
> > > >   {
> > > > LockGuard guard(m); // OK in C++ 2020, now deadlocks
> > > >   }
> > > > }
> > > > ```
> > > > ```
> > > > BinaryOperator 0x135036220 'int[3]' lvalue ','
> > > > |-CXXStaticCastExpr 0x1350361d0 'void' static_cast 
> > > > | `-CXXFunctionalCastExpr 0x135036198 'LockGuard':'struct LockGuard' 
> > > > functional cast to LockGuard 
> > > > |   `-CXXBindTemporaryExpr 0x135036178 'LockGuard':'struct LockGuard' 
> > > > (CXXTemporary 0x135036178)
> > > > | `-CXXConstructExpr 0x135036140 'LockGuard':'struct LockGuard' 
> > > > 'void (Mutex &)'
> > > > |   `-DeclRefExpr 0x135035e18 'Mutex':'class Mutex' lvalue Var 
> > > > 0x135035b40 'm' 'Mutex':'class Mutex'
> > > > `-DeclRefExpr 0x135036200 'int[3]' lvalue Var 0x135035928 'v' 'int[3]'
> > > > ```
> > > If `MaterializeTemporaryExpr` represents a "temporary materialization 
> > > conversion", then the above should already have one just under the 
> > > `static_cast` to `void` (since the cast operand would be a 
> > > discarded-value expression).
> > > 
> > > There may be unfortunate effects from materializing temporaries for 
> > > discarded-value expressions though: Technically, temporaries are also 
> > > created for objects having scalar type.
> > > 
> > > Currently, `MaterializeTemporaryExpr` is documented as being tied to 
> > > reference binding, but that is not correct: for example, 
> > > `MaterializeTemporaryExpr` also appears when a member access is made on a 
> > > temporary of class type.
> > http://eel.is/c++draft/class.temporary says:
> > ```
> > [Note 3: Temporary objects are materialized:
> > ...
> > (2.6)
> > when a prvalue that has type other than cv void appears as a 
> > discarded-value expression ([expr.context]).
> > — end note]
> > ```
> > Seems we should materialized the discard-value expression in this case, 
> > WDYT?
> I think we should, but what is the codegen fallout? Would no-opt builds start 
> writing `42` into allocated memory for `static_cast(42)`?
Thanks for your confirm @hubert.reinterpretcast ! 

I have tried locally, the generated  IR of `void f()` is:
```
define void @f()() {
entry:
  %v = alloca [3 x i32], align 4
  %m = alloca %class.Mutex, align 8
  %__range1 = alloca ptr, align 8
  %ref.tmp = alloca %struct.LockGuard, align 8
  %__begin1 = alloca ptr, align 8
  %__end1 = alloca ptr, align 8
  %x = alloca i32, align 4
  %guard = alloca 

[PATCH] D155217: [clang-tidy][include-cleaner] Don't warn for the same symbol twice

[Alex Brachet via Phabricator via cfe-commits]

abrachet created this revision.
abrachet added reviewers: VitaNuo, hokein.
Herald added subscribers: PiotrZSL, carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
abrachet requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Only one diagnostic per symbol is necessary, after that the diagnostics
can become overbearing.


https://reviews.llvm.org/D155217

Files:
  clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp
@@ -15,3 +15,5 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: no header providing "std::string" 
is directly included [misc-include-cleaner]
 int FooBarResult = foobar();
 // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: no header providing "foobar" is 
directly included [misc-include-cleaner]
+int FooBarResult2 = foobar();
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:
Index: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
@@ -106,7 +106,11 @@
   const SourceManager *SM = Result.SourceManager;
   const FileEntry *MainFile = SM->getFileEntryForID(SM->getMainFileID());
   llvm::DenseSet Used;
-  std::vector Missing;
+  using SymRef = include_cleaner::SymbolReference;
+  auto Cmp = [](const SymRef , const SymRef ) {
+return Left.Target.name() < Right.Target.name();
+  };
+  std::map Missing{Cmp};
   llvm::SmallVector MainFileDecls;
   for (Decl *D : Result.Nodes.getNodeAs("top")->decls()) {
 if (!SM->isWrittenInMainFile(SM->getExpansionLoc(D->getLocation(
@@ -135,7 +139,7 @@
  if (!Satisfied && !Providers.empty() &&
  Ref.RT == include_cleaner::RefType::Explicit &&
  !shouldIgnore(Providers.front()))
-   Missing.push_back({Ref, Providers.front()});
+   Missing.try_emplace(Ref, Providers.front());
});
 
   std::vector Unused;
@@ -181,9 +185,9 @@
 
   tooling::HeaderIncludes HeaderIncludes(getCurrentMainFile(), Code,
  FileStyle->IncludeStyle);
-  for (const auto  : Missing) {
+  for (const auto &[SymRef, Header] : Missing) {
 std::string Spelling =
-include_cleaner::spellHeader({Inc.Missing, *HS, MainFile});
+include_cleaner::spellHeader({Header, *HS, MainFile});
 bool Angled = llvm::StringRef{Spelling}.starts_with("<");
 // We might suggest insertion of an existing include in edge cases, e.g.,
 // include is present in a PP-disabled region, or spelling of the header
@@ -192,9 +196,9 @@
 if (auto Replacement =
 HeaderIncludes.insert(llvm::StringRef{Spelling}.trim("\"<>"),
   Angled, tooling::IncludeDirective::Include))
-  diag(SM->getSpellingLoc(Inc.SymRef.RefLocation),
+  diag(SM->getSpellingLoc(SymRef.RefLocation),
"no header providing \"%0\" is directly included")
-  << Inc.SymRef.Target.name()
+  << SymRef.Target.name()
   << FixItHint::CreateInsertion(
  SM->getComposedLoc(SM->getMainFileID(),
 Replacement->getOffset()),


Index: clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp
@@ -15,3 +15,5 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: no header providing "std::string" is directly included [misc-include-cleaner]
 int FooBarResult = foobar();
 // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: no header providing "foobar" is directly included [misc-include-cleaner]
+int FooBarResult2 = foobar();
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:
Index: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
@@ -106,7 +106,11 @@
   const SourceManager *SM = Result.SourceManager;
   const FileEntry *MainFile = SM->getFileEntryForID(SM->getMainFileID());
   llvm::DenseSet Used;
-  std::vector Missing;
+  using SymRef = include_cleaner::SymbolReference;
+  auto Cmp = [](const SymRef , const SymRef ) {
+return Left.Target.name() < Right.Target.name();
+  };
+  std::map Missing{Cmp};
   llvm::SmallVector MainFileDecls;
   for (Decl *D : 

[PATCH] D152632: [Clang] Add warnings for CWG2521

[PoYao Chang via Phabricator via cfe-commits]

rZhBoYao updated this revision to Diff 540072.
rZhBoYao marked 6 inline comments as done.
rZhBoYao edited the summary of this revision.
rZhBoYao added a comment.

-Wdeprecated-literal-operator will be on by default in 
https://reviews.llvm.org/D153156


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

https://reviews.llvm.org/D152632

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/IdentifierTable.h
  clang/lib/Basic/IdentifierTable.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/CXX/drs/dr25xx.cpp
  clang/www/cxx_dr_status.html

Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -14933,7 +14933,7 @@
 https://cplusplus.github.io/CWG/issues/2521.html;>2521
 DR
 User-defined literals and reserved identifiers
-Unknown
+Clang 17
   
   
 https://cplusplus.github.io/CWG/issues/2522.html;>2522
Index: clang/test/CXX/drs/dr25xx.cpp
===
--- clang/test/CXX/drs/dr25xx.cpp
+++ clang/test/CXX/drs/dr25xx.cpp
@@ -1,4 +1,14 @@
-// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-unknown %s -verify
+// RUN: %clang_cc1 -std=c++98 -triple x86_64-unknown-unknown %s -verify -fexceptions -fcxx-exceptions -pedantic-errors
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-unknown %s -verify -fexceptions -fcxx-exceptions -pedantic-errors
+// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown %s -verify -fexceptions -fcxx-exceptions -pedantic-errors
+// RUN: %clang_cc1 -std=c++17 -triple x86_64-unknown-unknown %s -verify -fexceptions -fcxx-exceptions -pedantic-errors
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-unknown %s -verify -fexceptions -fcxx-exceptions -pedantic-errors
+// RUN: %clang_cc1 -std=c++23 -triple x86_64-unknown-unknown %s -verify -fexceptions -fcxx-exceptions -pedantic-errors
+// RUN: %clang_cc1 -std=c++2c -triple x86_64-unknown-unknown %s -verify -fexceptions -fcxx-exceptions -pedantic-errors
+
+#if __cplusplus < 201103L
+// expected-no-diagnostics
+#endif
 
 namespace dr2516 { // dr2516: yes
// NB: reusing 1482 test
@@ -13,9 +23,13 @@
 
 namespace dr2518 { // dr2518: 17
 
+#if __cplusplus >= 201103L
 template 
 void f(T t) {
   if constexpr (sizeof(T) != sizeof(int)) {
+#if __cplusplus < 201703L
+// expected-error@-2 {{constexpr if is a C++17 extension}}
+#endif
 static_assert(false, "must be int-sized"); // expected-error {{must be int-size}}
   }
 }
@@ -28,6 +42,9 @@
 template 
 struct S {
   static_assert(false); // expected-error {{static assertion failed}}
+#if __cplusplus < 201703L
+// expected-error@-2 {{'static_assert' with no message is a C++17 extension}}
+#endif
 };
 
 template <>
@@ -41,11 +58,31 @@
   S s2;
   S s3; // expected-note {{in instantiation of template class 'dr2518::S' requested here}}
 }
+#endif
 
 }
 
+namespace dr2521 { // dr2521: 17
+#if __cplusplus >= 201103L
+#pragma clang diagnostic push
+#pragma clang diagnostic warning "-Wdeprecated-literal-operator"
+long double operator""  _\u03C0___(long double);
+// expected-warning@-1 {{identifier '_π___' preceded by whitespace in a literal operator declaration is deprecated}}
+// expected-warning@-2 {{user-defined literal suffixes containing '__' are reserved}}
+
+template  decltype(sizeof 0)
+operator""  _div();
+// expected-warning@-1 {{identifier '_div' preceded by whitespace in a literal operator declaration is deprecated}}
+
+using ::dr2521::operator"" _\u03C0___;
+using ::dr2521::operator""_div;
+// expected-warning@-2 {{identifier '_π___' preceded by whitespace in a literal operator declaration is deprecated}}
+#pragma clang diagnostic pop
+#endif
+} // namespace dr2521
 
 namespace dr2565 { // dr2565: 16 open
+#if __cplusplus >= 202002L
   template
 concept C = requires (typename T::type x) {
   x + 1;
@@ -107,4 +144,5 @@
   // expected-error@-1{{static assertion failed}}
   // expected-note@-2{{because substituted constraint expression is ill-formed: constraint depends on a previously diagnosed expression}}
 
+#endif
 }
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -502,13 +502,16 @@
 IdentifierInfo *II = Name.Identifier;
 ReservedIdentifierStatus Status = II->isReserved(PP.getLangOpts());
 SourceLocation Loc = Name.getEndLoc();
-if (isReservedInAllContexts(Status) &&
-!PP.getSourceManager().isInSystemHeader(Loc)) {
-  Diag(Loc, diag::warn_reserved_extern_symbol)
-  << II << static_cast(Status)
-  << FixItHint::CreateReplacement(
- Name.getSourceRange(),
- (StringRef("operator\"\"") + 

[PATCH] D155215: [clangd] Fix the range for include reference to itself.

[Viktoriia Bakalova via Phabricator via cfe-commits]

VitaNuo created this revision.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
VitaNuo requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155215

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -2299,7 +2299,7 @@
 
 TEST(FindReferences, UsedSymbolsFromInclude) {
   const char *Tests[] = {
-  R"cpp([[#include ^"bar.h"]]
+  R"cpp(   [[#include ^"bar.h"]]
 #include 
 int fstBar = [[bar1]]();
 int sndBar = [[bar2]]();
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -1363,9 +1363,11 @@
 
   // Add the #include line to the references list.
   auto IncludeLen = std::string{"#include"}.length() + Inc.Written.length() + 
1;
+  auto Start =
+  offsetToPosition(SM.getBufferData(SM.getMainFileID()), Inc.HashOffset);
   ReferencesResult::Reference Result;
-  Result.Loc.range = clangd::Range{Position{Inc.HashLine, 0},
-   Position{Inc.HashLine, (int)IncludeLen}};
+  Result.Loc.range = clangd::Range{
+  Start, Position{Inc.HashLine, Start.character + (int)IncludeLen}};
   Result.Loc.uri = URIMainFile;
   Results.References.push_back(std::move(Result));
 


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -2299,7 +2299,7 @@
 
 TEST(FindReferences, UsedSymbolsFromInclude) {
   const char *Tests[] = {
-  R"cpp([[#include ^"bar.h"]]
+  R"cpp(   [[#include ^"bar.h"]]
 #include 
 int fstBar = [[bar1]]();
 int sndBar = [[bar2]]();
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -1363,9 +1363,11 @@
 
   // Add the #include line to the references list.
   auto IncludeLen = std::string{"#include"}.length() + Inc.Written.length() + 1;
+  auto Start =
+  offsetToPosition(SM.getBufferData(SM.getMainFileID()), Inc.HashOffset);
   ReferencesResult::Reference Result;
-  Result.Loc.range = clangd::Range{Position{Inc.HashLine, 0},
-   Position{Inc.HashLine, (int)IncludeLen}};
+  Result.Loc.range = clangd::Range{
+  Start, Position{Inc.HashLine, Start.character + (int)IncludeLen}};
   Result.Loc.uri = URIMainFile;
   Results.References.push_back(std::move(Result));
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154576: [RISCV] RISCV vector calling convention (1/2)

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/AST/ItaniumMangle.cpp:3267
   case CC_PreserveAll:
+  case CC_RISCVVectorCall:
 // FIXME: we should be mangling all of the above.

Is it possible to use this calling convention on Windows where we'd hit the 
Microsoft name mangler?



Comment at: clang/test/CodeGen/RISCV/riscv-vector-cc-attr.c:1
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// REQUIRES: riscv-registered-target

You should also have Sema tests for various things (attribute accepts no 
arguments, only applies to functions, does/doesn't silently convert to cdecl, 
etc). You should also have some tests using the attribute as a type attribute 
instead of a declaration attribute.

You should also have C++ tests for Sema and codegen for things like putting the 
calling convention on a member function or a lambda to ensure those do 
reasonable things with the convention.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154576

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

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

[Fangrui Song 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 rGd986462b5053: [Driver] Recognize powerpc-unknown-eabi as a 
bare-metal toolchain (authored by cwalther, committed by MaskRay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154357

Files:
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/test/Driver/baremetal.cpp


Index: clang/test/Driver/baremetal.cpp
===
--- clang/test/Driver/baremetal.cpp
+++ clang/test/Driver/baremetal.cpp
@@ -345,6 +345,58 @@
 // CHECK-RV32IMAFC-SAME: 
"-L[[SYSROOT:[^"]+]]{{[/\\]+}}rv32imafc{{[/\\]+}}ilp32f{{[/\\]+}}lib"
 // CHECK-RV32IMAFC-SAME: 
"-L[[RESOURCE_DIR:[^"]+]]{{[/\\]+}}lib{{[/\\]+}}baremetal{{[/\\]+}}rv32imafc{{[/\\]+}}ilp32f"
 
+// RUN: %clang %s -### --target=powerpc-unknown-eabi 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-PPCEABI %s
+// CHECK-PPCEABI: InstalledDir: [[INSTALLEDDIR:.+]]
+// CHECK-PPCEABI: "-nostdsysteminc"
+// CHECK-PPCEABI-SAME: "-resource-dir" "[[RESOURCE:[^"]+]]"
+// CHECK-PPCEABI-SAME: "-internal-isystem" 
"[[INSTALLEDDIR]]{{[/\\]+}}..{{[/\\]+}}lib{{[/\\]+}}clang-runtimes{{[/\\]+[^"]*}}include{{[/\\]+}}c++{{[/\\]+}}v1"
+// CHECK-PPCEABI-SAME: "-internal-isystem" "[[RESOURCE]]{{[/\\]+}}include"
+// CHECK-PPCEABI-SAME: "-internal-isystem" 
"[[INSTALLEDDIR]]{{[/\\]+}}..{{[/\\]+}}lib{{[/\\]+}}clang-runtimes{{[/\\]+[^"]*}}include"
+// CHECK-PPCEABI-NEXT: ld{{(.exe)?}}" "{{.*}}.o" "-Bstatic"
+// CHECK-PPCEABI-SAME: 
"-L[[INSTALLEDDIR]]{{[/\\]+}}..{{[/\\]+}}lib{{[/\\]+}}clang-runtimes{{[/\\]+[^"]*}}lib"
+// CHECK-PPCEABI-SAME: "-L[[RESOURCE]]{{[/\\]+}}lib{{[/\\]+}}baremetal"
+// CHECK-PPCEABI-SAME: "-lc" "-lm" "-lclang_rt.builtins-powerpc" "-o" "a.out"
+
+// RUN: %clang %s -### --target=powerpc64-unknown-eabi 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-PPC64EABI %s
+// CHECK-PPC64EABI: InstalledDir: [[INSTALLEDDIR:.+]]
+// CHECK-PPC64EABI: "-nostdsysteminc"
+// CHECK-PPC64EABI-SAME: "-resource-dir" "[[RESOURCE:[^"]+]]"
+// CHECK-PPC64EABI-SAME: "-internal-isystem" 
"[[INSTALLEDDIR]]{{[/\\]+}}..{{[/\\]+}}lib{{[/\\]+}}clang-runtimes{{[/\\]+[^"]*}}include{{[/\\]+}}c++{{[/\\]+}}v1"
+// CHECK-PPC64EABI-SAME: "-internal-isystem" "[[RESOURCE]]{{[/\\]+}}include"
+// CHECK-PPC64EABI-SAME: "-internal-isystem" 
"[[INSTALLEDDIR]]{{[/\\]+}}..{{[/\\]+}}lib{{[/\\]+}}clang-runtimes{{[/\\]+[^"]*}}include"
+// CHECK-PPC64EABI-NEXT: ld{{(.exe)?}}" "{{.*}}.o" "-Bstatic"
+// CHECK-PPC64EABI-SAME: 
"-L[[INSTALLEDDIR]]{{[/\\]+}}..{{[/\\]+}}lib{{[/\\]+}}clang-runtimes{{[/\\]+[^"]*}}lib"
+// CHECK-PPC64EABI-SAME: "-L[[RESOURCE]]{{[/\\]+}}lib{{[/\\]+}}baremetal"
+// CHECK-PPC64EABI-SAME: "-lc" "-lm" "-lclang_rt.builtins-powerpc64" "-o" 
"a.out"
+
+// RUN: %clang %s -### --target=powerpcle-unknown-eabi 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-PPCLEEABI %s
+// CHECK-PPCLEEABI: InstalledDir: [[INSTALLEDDIR:.+]]
+// CHECK-PPCLEEABI: "-nostdsysteminc"
+// CHECK-PPCLEEABI-SAME: "-resource-dir" "[[RESOURCE:[^"]+]]"
+// CHECK-PPCLEEABI-SAME: "-internal-isystem" 
"[[INSTALLEDDIR]]{{[/\\]+}}..{{[/\\]+}}lib{{[/\\]+}}clang-runtimes{{[/\\]+[^"]*}}include{{[/\\]+}}c++{{[/\\]+}}v1"
+// CHECK-PPCLEEABI-SAME: "-internal-isystem" "[[RESOURCE]]{{[/\\]+}}include"
+// CHECK-PPCLEEABI-SAME: "-internal-isystem" 
"[[INSTALLEDDIR]]{{[/\\]+}}..{{[/\\]+}}lib{{[/\\]+}}clang-runtimes{{[/\\]+[^"]*}}include"
+// CHECK-PPCLEEABI-NEXT: ld{{(.exe)?}}" "{{.*}}.o" "-Bstatic"
+// CHECK-PPCLEEABI-SAME: 
"-L[[INSTALLEDDIR]]{{[/\\]+}}..{{[/\\]+}}lib{{[/\\]+}}clang-runtimes{{[/\\]+[^"]*}}lib"
+// CHECK-PPCLEEABI-SAME: "-L[[RESOURCE]]{{[/\\]+}}lib{{[/\\]+}}baremetal"
+// CHECK-PPCLEEABI-SAME: "-lc" "-lm" "-lclang_rt.builtins-powerpcle" "-o" 
"a.out"
+
+// RUN: %clang %s -### --target=powerpc64le-unknown-eabi 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-PPC64LEEABI %s
+// CHECK-PPC64LEEABI: InstalledDir: [[INSTALLEDDIR:.+]]
+// CHECK-PPC64LEEABI: "-nostdsysteminc"
+// CHECK-PPC64LEEABI-SAME: "-resource-dir" "[[RESOURCE:[^"]+]]"
+// CHECK-PPC64LEEABI-SAME: "-internal-isystem" 
"[[INSTALLEDDIR]]{{[/\\]+}}..{{[/\\]+}}lib{{[/\\]+}}clang-runtimes{{[/\\]+[^"]*}}include{{[/\\]+}}c++{{[/\\]+}}v1"
+// CHECK-PPC64LEEABI-SAME: "-internal-isystem" "[[RESOURCE]]{{[/\\]+}}include"
+// CHECK-PPC64LEEABI-SAME: "-internal-isystem" 
"[[INSTALLEDDIR]]{{[/\\]+}}..{{[/\\]+}}lib{{[/\\]+}}clang-runtimes{{[/\\]+[^"]*}}include"
+// CHECK-PPC64LEEABI-NEXT: ld{{(.exe)?}}" "{{.*}}.o" "-Bstatic"
+// CHECK-PPC64LEEABI-SAME: 
"-L[[INSTALLEDDIR]]{{[/\\]+}}..{{[/\\]+}}lib{{[/\\]+}}clang-runtimes{{[/\\]+[^"]*}}lib"
+// CHECK-PPC64LEEABI-SAME: "-L[[RESOURCE]]{{[/\\]+}}lib{{[/\\]+}}baremetal"
+// CHECK-PPC64LEEABI-SAME: "-lc" "-lm" "-lclang_rt.builtins-powerpc64le" "-o" 
"a.out"
+
 // Check that compiler-rt library without the arch filename suffix will
 // be used if present.
 // 

[clang] d986462 - [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

[Fangrui Song via cfe-commits]

Author: Christian Walther
Date: 2023-07-13T09:19:25-07:00
New Revision: d986462b50536082065a44d12eeed1d0e1539976

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

LOG: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

This seems to match https://gcc.gnu.org/install/specific.html#powerpc-x-eabi

It seems that anything with OS `none` (although that doesn’t seem to be 
distinguished from `unknown`) or with environment `eabi` should be treated as 
bare-metal.
Since this seems to have been handled on a case-by-case basis in the past 
([arm](https://reviews.llvm.org/D33259), 
[riscv](https://reviews.llvm.org/D91442), 
[aarch64](https://reviews.llvm.org/D34)), what I am proposing here is to 
add another case to the list to also handle 
`powerpc[64][le]-unknown-unknown-eabi` using the `BareMetal` toolchain, 
following the example of the existing cases. (We don’t care about powerpc64 and 
powerpc[64]le, but it seemed appropriate to lump them in.)

At Indel, we have been building bare-metal embedded applications that run on 
custom PowerPC and ARM systems with Clang and LLD for a couple of years now, 
using target triples `powerpc-indel-eabi`, `powerpc-indel-eabi750`, 
`arm-indel-eabi`, `aarch64-indel-eabi` (which I just learned from D153430 is 
wrong and should be `aarch64-indel-elf` instead, but that’s a different 
matter). This has worked fine for ARM, but for PowerPC we have been unable to 
call the linker (LLD) through the Clang driver, because it would insist on 
calling GCC as the linker, even when told `-fuse-ld=lld`. That does not work 
for us, there is no GCC around. Instead we had to call `ld.lld` directly, 
introducing some special cases in our build system to translate between 
linker-via-driver and linker-called-directly command line arguments. I have now 
dug into why that is, and found that the difference between ARM and PowerPC is 
that `arm-indel-eabi` hits a special case that causes the Clang driver to 
instantiate a `BareMetal` toolchain that is able to call LLD and works the way 
we need, whereas `powerpc-indel-eabi` lands in the default case of a 
`Generic_ELF` (subclass of `Generic_GCC`) toolchain which expects GCC.

Reviewed By: MaskRay, michaelplatings, #powerpc, nemanjai

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/BareMetal.cpp
clang/test/Driver/baremetal.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/BareMetal.cpp 
b/clang/lib/Driver/ToolChains/BareMetal.cpp
index 850d7c59c3474a..a4bd4bb0f6fb76 100644
--- a/clang/lib/Driver/ToolChains/BareMetal.cpp
+++ b/clang/lib/Driver/ToolChains/BareMetal.cpp
@@ -161,6 +161,12 @@ static bool isRISCVBareMetal(const llvm::Triple ) {
   return Triple.getEnvironmentName() == "elf";
 }
 
+/// Is the triple powerpc[64][le]-*-none-eabi?
+static bool isPPCBareMetal(const llvm::Triple ) {
+  return Triple.isPPC() && Triple.getOS() == llvm::Triple::UnknownOS &&
+ Triple.getEnvironment() == llvm::Triple::EABI;
+}
+
 static void findMultilibsFromYAML(const ToolChain , const Driver ,
   StringRef MultilibPath, const ArgList ,
   DetectedMultilibs ) {
@@ -226,7 +232,7 @@ void BareMetal::findMultilibs(const Driver , const 
llvm::Triple ,
 
 bool BareMetal::handlesTarget(const llvm::Triple ) {
   return isARMBareMetal(Triple) || isAArch64BareMetal(Triple) ||
- isRISCVBareMetal(Triple);
+ isRISCVBareMetal(Triple) || isPPCBareMetal(Triple);
 }
 
 Tool *BareMetal::buildLinker() const {

diff  --git a/clang/test/Driver/baremetal.cpp b/clang/test/Driver/baremetal.cpp
index 41122f663e49f4..b426e20265ecb2 100644
--- a/clang/test/Driver/baremetal.cpp
+++ b/clang/test/Driver/baremetal.cpp
@@ -345,6 +345,58 @@
 // CHECK-RV32IMAFC-SAME: 
"-L[[SYSROOT:[^"]+]]{{[/\\]+}}rv32imafc{{[/\\]+}}ilp32f{{[/\\]+}}lib"
 // CHECK-RV32IMAFC-SAME: 
"-L[[RESOURCE_DIR:[^"]+]]{{[/\\]+}}lib{{[/\\]+}}baremetal{{[/\\]+}}rv32imafc{{[/\\]+}}ilp32f"
 
+// RUN: %clang %s -### --target=powerpc-unknown-eabi 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-PPCEABI %s
+// CHECK-PPCEABI: InstalledDir: [[INSTALLEDDIR:.+]]
+// CHECK-PPCEABI: "-nostdsysteminc"
+// CHECK-PPCEABI-SAME: "-resource-dir" "[[RESOURCE:[^"]+]]"
+// CHECK-PPCEABI-SAME: "-internal-isystem" 
"[[INSTALLEDDIR]]{{[/\\]+}}..{{[/\\]+}}lib{{[/\\]+}}clang-runtimes{{[/\\]+[^"]*}}include{{[/\\]+}}c++{{[/\\]+}}v1"
+// CHECK-PPCEABI-SAME: "-internal-isystem" "[[RESOURCE]]{{[/\\]+}}include"
+// CHECK-PPCEABI-SAME: "-internal-isystem" 
"[[INSTALLEDDIR]]{{[/\\]+}}..{{[/\\]+}}lib{{[/\\]+}}clang-runtimes{{[/\\]+[^"]*}}include"
+// CHECK-PPCEABI-NEXT: ld{{(.exe)?}}" "{{.*}}.o" "-Bstatic"
+// CHECK-PPCEABI-SAME: 

[PATCH] D155213: [HIP] Add `-fno-hip-uniform-block`

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:7226
+  } else {
+Args.claimAllArgs(options::OPT_fhip_uniform_block,
+  options::OPT_fno_hip_uniform_block);

MaskRay wrote:
> Why is the -Wunused-command-line-argument warning suppressed in non-IsHIP 
> mode?
Users may want to add these options to clang config file.

Is there a general rule which options should be claimed?


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

https://reviews.llvm.org/D155213

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

[PATCH] D155213: [HIP] Add `-fno-hip-uniform-block`

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:7226
+  } else {
+Args.claimAllArgs(options::OPT_fhip_uniform_block,
+  options::OPT_fno_hip_uniform_block);

Why is the -Wunused-command-line-argument warning suppressed in non-IsHIP mode?


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

https://reviews.llvm.org/D155213

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

[PATCH] D155213: [HIP] Add `-fno-hip-uniform-block`

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: tra, b-sumner, MaskRay, arsenm, scchan.
Herald added subscribers: jdoerfert, kerbowa, jvesely.
Herald added a project: All.
yaxunl requested review of this revision.
Herald added a subscriber: wdng.

By default, clang assumes HIP kernels are launched with uniform block size,
which is the case for kernels launched through triple chevron or
hipLaunchKernelGGL. Clang adds uniform-work-group-size function attribute
to HIP kernels to allow the backend to do optimizations on that.

However, in some rare cases, HIP kernels can be launched
through hipExtModuleLaunchKernel where global work size is specified,
which may result in non-uniform block size.

To be able to support non-uniform block size for HIP kernels,
an option `-f[no-]hip-uniform-block is added. By default it is on.


https://reviews.llvm.org/D155213

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/Targets/AMDGPU.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGenHIP/default-attributes.hip
  clang/test/Driver/hip-options.hip

Index: clang/test/Driver/hip-options.hip
===
--- clang/test/Driver/hip-options.hip
+++ clang/test/Driver/hip-options.hip
@@ -169,3 +169,28 @@
 // RUN: %clang -### -nogpuinc -nogpulib -fhip-fp32-correctly-rounded-divide-sqrt \
 // RUN:   --cuda-gpu-arch=gfx906  %s 2>&1 | FileCheck -check-prefixes=CRDS %s
 // CRDS-NOT: "-f{{(no-)?}}hip-fp32-correctly-rounded-divide-sqrt"
+
+// Check -fno-hip-uniform-block is passed to clang -cc1 but
+// (default) -fhip-uniform-block is not.
+
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu -nogpuinc -nogpulib -fno-hip-uniform-block \
+// RUN:   --cuda-gpu-arch=gfx906 %s 2>&1 | FileCheck -check-prefix=NOUNIBLK %s
+
+// NOUNIBLK: "-cc1"{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} "-fno-hip-uniform-block"
+// NOUNIBLK: "-cc1"{{.*}} "-triple" "x86_64-unknown-linux-gnu" {{.*}} "-fno-hip-uniform-block"
+
+// RUN: %clang -### -nogpuinc -nogpulib -fhip-uniform-block \
+// RUN:   --cuda-gpu-arch=gfx906 %s 2>&1 | FileCheck -check-prefix=UNIBLK %s
+
+// RUN: %clang -### -nogpuinc -nogpulib \
+// RUN:   --cuda-gpu-arch=gfx906 %s 2>&1 | FileCheck -check-prefix=UNIBLK %s
+
+// UNIBLK-NOT: "-f{{(no-)?}}hip-uniform-block"
+
+// Check no warnings for -f[no-]uniform-block.
+
+// RUN: %clang -fdriver-only -Werror --target=x86_64-unknown-linux-gnu -nogpuinc -nogpulib -fno-hip-uniform-block \
+// RUN:   -fhip-uniform-block --cuda-gpu-arch=gfx906 %s 2>&1 | count 0
+
+// RUN: %clang  -fdriver-only -Werror --target=x86_64-unknown-linux-gnu -nostdinc -nostdlib -fno-hip-uniform-block \
+// RUN:   -fhip-uniform-block -x c++ %s 2>&1 | count 0
Index: clang/test/CodeGenHIP/default-attributes.hip
===
--- clang/test/CodeGenHIP/default-attributes.hip
+++ clang/test/CodeGenHIP/default-attributes.hip
@@ -5,6 +5,9 @@
 // RUN: %clang_cc1 -O3 -triple amdgcn-amd-amdhsa -x hip -fno-ident -fcuda-is-device \
 // RUN:-emit-llvm -o - %s | FileCheck -check-prefix=OPT %s
 
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -x hip -fno-ident -fcuda-is-device -fno-hip-uniform-block \
+// RUN:-emit-llvm -o - %s | FileCheck -check-prefix=NOUNIBLK %s
+
 #define __device__ __attribute__((device))
 #define __global__ __attribute__((global))
 
@@ -20,6 +23,12 @@
 // OPT-NEXT:  entry:
 // OPT-NEXT:ret void
 //
+// NOUNIBLK: Function Attrs: convergent mustprogress noinline nounwind optnone
+// NOUNIBLK-LABEL: define {{[^@]+}}@_Z4funcv
+// NOUNIBLK-SAME: () #[[ATTR0:[0-9]+]] {
+// NOUNIBLK-NEXT:  entry:
+// NOUNIBLK-NEXT:ret void
+//
 __device__ void func() {
 
 }
@@ -36,21 +45,34 @@
 // OPT-NEXT:  entry:
 // OPT-NEXT:ret void
 //
+// NOUNIBLK: Function Attrs: convergent mustprogress noinline norecurse nounwind optnone
+// NOUNIBLK-LABEL: define {{[^@]+}}@_Z6kernelv
+// NOUNIBLK-SAME: () #[[ATTR1:[0-9]+]] {
+// NOUNIBLK-NEXT:  entry:
+// NOUNIBLK-NEXT:ret void
+//
 __global__ void kernel() {
 
 }
 //.
-// OPTNONE: attributes #0 = { convergent mustprogress noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" }
-// OPTNONE: attributes #1 = { convergent mustprogress noinline norecurse nounwind optnone "amdgpu-flat-work-group-size"="1,1024" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "uniform-work-group-size"="true" }
+// OPTNONE: attributes #[[ATTR0]] = { convergent mustprogress noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" }
+// OPTNONE: attributes #[[ATTR1]] = { convergent mustprogress noinline norecurse nounwind optnone "amdgpu-flat-work-group-size"="1,1024" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "uniform-work-group-size"="true" }
+//.
+// OPT: attributes #[[ATTR0]] = { mustprogress nofree norecurse nosync nounwind willreturn 

[PATCH] D150221: Add option -fkeep-persistent-storage-variables to emit all variables that have a persistent storage duration

[Hubert Tong via Phabricator via cfe-commits]

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

> Furthermore, there is some discussion over covering more than just variables, 
> but also artifacts with reasonable mangled names such as the symbols for 
> temporaries bound to references with "persistent storage" and guard variables.

We can follow up with a separate patch to add the extra functionality. This 
patch LGTM for the scope it covers.




Comment at: clang/include/clang/Driver/Options.td:1703
+  PosFlag, NegFlag,
+  BothFlags<[NoXarchOption], " keeping all variables that have a persistent 
storage duration, including global, static and thread local variables, to 
guarantee that they can be directly addressed">>;
 defm fixed_point : BoolFOption<"fixed-point",

Minor nit: Use hyphen in "thread-local".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150221

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

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

[Christian Walther via Phabricator via cfe-commits]

cwalther added a comment.

Christian Walther 


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

https://reviews.llvm.org/D154357

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

[PATCH] D150226: [Clang] Remove ability to downgrade warning on the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D150226#4461846 , @efriedma wrote:

> The fundamental problem here is the interaction with SFINAE; if we don't 
> diagnose this as an error, we actually miscompile some cases.  Because of 
> that, a flag wouldn't really be "turning off the warning"; it would be 
> enabling a non-compliant language mode that has different rules for whether 
> enum casts are legal.
>
> If it weren't for that, I don't think anyone would be in a hurry to turn the 
> warning into a hard error.

+1 to this. I'm worried about the miscompiles continuing in the wild. We did 
the best we could when landing the warning-as-error changes in D131307 
 by telling users explicitly "This diagnostic 
is expected to turn into an error-only diagnostic in the next Clang release." 
in the release notes. Obviously, that's not ideal because very few folks read 
release notes, but we don't have any better mechanism for communicating these 
kinds of changes.

What do folks think is a reasonable path forward here? Given that we're going 
to branch the Clang 17 release in a few weeks (AIUI), my suggestion is to kick 
the can down the road by landing these changes just after we branch. That way 
the changes land such that early adopters can test and see how disruptive we're 
being without time pressure or hassle of dealing with release branches. Do 
folks think that's a reasonable approach? (I'm open to other suggestions.)


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

https://reviews.llvm.org/D150226

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

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D154357#4497973 , @cwalther wrote:

> Okay. No, I don’t have write access (as far as I know) and would appreciate 
> if you (or any other maintainer) could land the patch, with any commit 
> message you deem appropriate.

Happy to do this! What `git commit --amend --author=...` shall I use to set the 
author name/email for the commit?


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

https://reviews.llvm.org/D154357

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

[PATCH] D155211: [NVPTX] Add initial support for '.alias' in PTX

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 updated this revision to Diff 540062.
jhuber6 added a comment.

Remove changes in `clang` that I forgot to remove to keep this restricted to 
the codegen. Afterwards we can remove the sema in clang and test it there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155211

Files:
  llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
  llvm/lib/Target/NVPTX/NVPTXAsmPrinter.h
  llvm/test/CodeGen/NVPTX/alias-errors.ll
  llvm/test/CodeGen/NVPTX/alias.ll

Index: llvm/test/CodeGen/NVPTX/alias.ll
===
--- llvm/test/CodeGen/NVPTX/alias.ll
+++ llvm/test/CodeGen/NVPTX/alias.ll
@@ -1,7 +1,27 @@
-; RUN: not --crash llc < %s -march=nvptx -mcpu=sm_20 2>&1 | FileCheck %s
-
-; Check that llc dies gracefully when given an alias.
+; RUN: llc < %s -march=nvptx64 -mcpu=sm_30 -mattr=+ptx63 | FileCheck %s
 
 define i32 @a() { ret i32 0 }
-; CHECK: ERROR: Module has aliases
 @b = internal alias i32 (), ptr @a
+@c = internal alias i32 (), ptr @a
+
+define void @foo(i32 %0, ptr %1) { ret void }
+@bar = alias i32 (), ptr @foo
+
+; CHECK: .visible .func  (.param .b32 func_retval0) a()
+
+;  CHECK: .visible .func foo(
+; CHECK-NEXT: .param .b32 foo_param_0,
+; CHECK-NEXT: .param .b64 foo_param_1
+; CHECK-NEXT: )
+
+;  CHECK: .visible .func  (.param .b32 func_retval0) b();
+; CHECK-NEXT: .alias b, a;
+
+;  CHECK: .visible .func  (.param .b32 func_retval0) c();
+; CHECK-NEXT: .alias c, a;
+
+;  CHECK: .visible .func bar(
+; CHECK-NEXT: .param .b32 foo_param_0,
+; CHECK-NEXT: .param .b64 foo_param_1
+; CHECK-NEXT: );
+; CHECK-NEXT: .alias bar, foo;
Index: llvm/test/CodeGen/NVPTX/alias-errors.ll
===
--- /dev/null
+++ llvm/test/CodeGen/NVPTX/alias-errors.ll
@@ -0,0 +1,9 @@
+; RUN: not --crash llc < %s -march=nvptx64 -mcpu=sm_30 -mattr=+ptx43 2>&1 | FileCheck %s --check-prefix=ATTR
+; RUN: not --crash llc < %s -march=nvptx64 -mcpu=sm_20 -mattr=+ptx63 2>&1 | FileCheck %s --check-prefix=ATTR
+; RUN: not --crash llc < %s -march=nvptx64 -mcpu=sm_30 -mattr=+ptx63 2>&1 | FileCheck %s --check-prefix=ALIAS
+
+; ATTR: .alias requires PTX version >= 6.3 and sm_30
+
+; ALIAS: NVPTX aliasee must be a non-kernel function
+@a = global i32 42, align 8
+@b = internal alias i32, ptr @a
Index: llvm/lib/Target/NVPTX/NVPTXAsmPrinter.h
===
--- llvm/lib/Target/NVPTX/NVPTXAsmPrinter.h
+++ llvm/lib/Target/NVPTX/NVPTXAsmPrinter.h
@@ -174,6 +174,7 @@
   void printModuleLevelGV(const GlobalVariable *GVar, raw_ostream ,
   bool processDemoted, const NVPTXSubtarget );
   void emitGlobals(const Module );
+  void emitGlobalAlias(const Module , const GlobalAlias );
   void emitHeader(Module , raw_ostream , const NVPTXSubtarget );
   void emitKernelFunctionDirectives(const Function , raw_ostream ) const;
   void emitVirtualRegister(unsigned int vr, raw_ostream &);
Index: llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
===
--- llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
+++ llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
@@ -473,6 +473,7 @@
   CurrentFnSym->print(O, MAI);
 
   emitFunctionParamList(F, O);
+  O << "\n";
 
   if (isKernelFunction(*F))
 emitKernelFunctionDirectives(*F, O);
@@ -623,6 +624,7 @@
   getSymbol(F)->print(O, MAI);
   O << "\n";
   emitFunctionParamList(F, O);
+  O << "\n";
   if (shouldEmitPTXNoReturn(F, TM))
 O << ".noreturn";
   O << ";\n";
@@ -790,10 +792,12 @@
 }
 
 bool NVPTXAsmPrinter::doInitialization(Module ) {
-  if (M.alias_size()) {
-report_fatal_error("Module has aliases, which NVPTX does not support.");
-return true; // error
-  }
+  const NVPTXTargetMachine  = static_cast(TM);
+  const NVPTXSubtarget  =
+  *static_cast(NTM.getSubtargetImpl());
+  if (M.alias_size() && (STI.getPTXVersion() < 63 || STI.getSmVersion() < 30))
+report_fatal_error(".alias requires PTX version >= 6.3 and sm_30");
+
   if (!isEmptyXXStructor(M.getNamedGlobal("llvm.global_ctors")) &&
   !LowerCtorDtor) {
 report_fatal_error(
@@ -850,6 +854,32 @@
   OutStreamer->emitRawText(OS2.str());
 }
 
+void NVPTXAsmPrinter::emitGlobalAlias(const Module , const GlobalAlias ) {
+  SmallString<128> Str;
+  raw_svector_ostream OS(Str);
+
+  MCSymbol *Name = getSymbol();
+  const Function *F = dyn_cast(GA.getAliasee());
+  if (!F || isKernelFunction(*F))
+report_fatal_error("NVPTX aliasee must be a non-kernel function");
+
+  if (GA.hasLinkOnceLinkage() || GA.hasWeakLinkage() ||
+  GA.hasAvailableExternallyLinkage() || GA.hasCommonLinkage())
+report_fatal_error("NVPTX aliasee must not be '.weak'");
+
+  OS << "\n";
+  emitLinkageDirective(F, OS);
+  OS << ".func ";
+  printReturnValStr(F, OS);
+  OS << Name->getName();
+  

[PATCH] D155211: [NVPTX] Add initial support for '.alias' in PTX

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 created this revision.
jhuber6 added reviewers: tra, arsenm, jlebar, kushanam.
Herald added subscribers: mattd, gchakrabarti, asavonic, jeroen.dobbelaere, 
hiraditya.
Herald added a reviewer: aaron.ballman.
Herald added a project: All.
jhuber6 requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, wangpc, wdng, jholewinski.
Herald added projects: clang, LLVM.

This patch adds initial support for using aliases when targeting PTX. We
perform a pretty strict conversion from the globals referenced to the
expected output.

These cannot currently be used due to a bug in the `nvlink`
implementation that causes aliases to pruned functions to crash the
linker.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155211

Files:
  clang/lib/Sema/SemaDeclAttr.cpp
  llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
  llvm/lib/Target/NVPTX/NVPTXAsmPrinter.h
  llvm/test/CodeGen/NVPTX/alias-errors.ll
  llvm/test/CodeGen/NVPTX/alias.ll

Index: llvm/test/CodeGen/NVPTX/alias.ll
===
--- llvm/test/CodeGen/NVPTX/alias.ll
+++ llvm/test/CodeGen/NVPTX/alias.ll
@@ -1,7 +1,27 @@
-; RUN: not --crash llc < %s -march=nvptx -mcpu=sm_20 2>&1 | FileCheck %s
-
-; Check that llc dies gracefully when given an alias.
+; RUN: llc < %s -march=nvptx64 -mcpu=sm_30 -mattr=+ptx63 | FileCheck %s
 
 define i32 @a() { ret i32 0 }
-; CHECK: ERROR: Module has aliases
 @b = internal alias i32 (), ptr @a
+@c = internal alias i32 (), ptr @a
+
+define void @foo(i32 %0, ptr %1) { ret void }
+@bar = alias i32 (), ptr @foo
+
+; CHECK: .visible .func  (.param .b32 func_retval0) a()
+
+;  CHECK: .visible .func foo(
+; CHECK-NEXT: .param .b32 foo_param_0,
+; CHECK-NEXT: .param .b64 foo_param_1
+; CHECK-NEXT: )
+
+;  CHECK: .visible .func  (.param .b32 func_retval0) b();
+; CHECK-NEXT: .alias b, a;
+
+;  CHECK: .visible .func  (.param .b32 func_retval0) c();
+; CHECK-NEXT: .alias c, a;
+
+;  CHECK: .visible .func bar(
+; CHECK-NEXT: .param .b32 foo_param_0,
+; CHECK-NEXT: .param .b64 foo_param_1
+; CHECK-NEXT: );
+; CHECK-NEXT: .alias bar, foo;
Index: llvm/test/CodeGen/NVPTX/alias-errors.ll
===
--- /dev/null
+++ llvm/test/CodeGen/NVPTX/alias-errors.ll
@@ -0,0 +1,9 @@
+; RUN: not --crash llc < %s -march=nvptx64 -mcpu=sm_30 -mattr=+ptx43 2>&1 | FileCheck %s --check-prefix=ATTR
+; RUN: not --crash llc < %s -march=nvptx64 -mcpu=sm_20 -mattr=+ptx63 2>&1 | FileCheck %s --check-prefix=ATTR
+; RUN: not --crash llc < %s -march=nvptx64 -mcpu=sm_30 -mattr=+ptx63 2>&1 | FileCheck %s --check-prefix=ALIAS
+
+; ATTR: .alias requires PTX version >= 6.3 and sm_30
+
+; ALIAS: NVPTX aliasee must be a non-kernel function
+@a = global i32 42, align 8
+@b = internal alias i32, ptr @a
Index: llvm/lib/Target/NVPTX/NVPTXAsmPrinter.h
===
--- llvm/lib/Target/NVPTX/NVPTXAsmPrinter.h
+++ llvm/lib/Target/NVPTX/NVPTXAsmPrinter.h
@@ -174,6 +174,7 @@
   void printModuleLevelGV(const GlobalVariable *GVar, raw_ostream ,
   bool processDemoted, const NVPTXSubtarget );
   void emitGlobals(const Module );
+  void emitGlobalAlias(const Module , const GlobalAlias );
   void emitHeader(Module , raw_ostream , const NVPTXSubtarget );
   void emitKernelFunctionDirectives(const Function , raw_ostream ) const;
   void emitVirtualRegister(unsigned int vr, raw_ostream &);
Index: llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
===
--- llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
+++ llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
@@ -473,6 +473,7 @@
   CurrentFnSym->print(O, MAI);
 
   emitFunctionParamList(F, O);
+  O << "\n";
 
   if (isKernelFunction(*F))
 emitKernelFunctionDirectives(*F, O);
@@ -623,6 +624,7 @@
   getSymbol(F)->print(O, MAI);
   O << "\n";
   emitFunctionParamList(F, O);
+  O << "\n";
   if (shouldEmitPTXNoReturn(F, TM))
 O << ".noreturn";
   O << ";\n";
@@ -790,10 +792,12 @@
 }
 
 bool NVPTXAsmPrinter::doInitialization(Module ) {
-  if (M.alias_size()) {
-report_fatal_error("Module has aliases, which NVPTX does not support.");
-return true; // error
-  }
+  const NVPTXTargetMachine  = static_cast(TM);
+  const NVPTXSubtarget  =
+  *static_cast(NTM.getSubtargetImpl());
+  if (M.alias_size() && (STI.getPTXVersion() < 63 || STI.getSmVersion() < 30))
+report_fatal_error(".alias requires PTX version >= 6.3 and sm_30");
+
   if (!isEmptyXXStructor(M.getNamedGlobal("llvm.global_ctors")) &&
   !LowerCtorDtor) {
 report_fatal_error(
@@ -850,6 +854,32 @@
   OutStreamer->emitRawText(OS2.str());
 }
 
+void NVPTXAsmPrinter::emitGlobalAlias(const Module , const GlobalAlias ) {
+  SmallString<128> Str;
+  raw_svector_ostream OS(Str);
+
+  MCSymbol *Name = getSymbol();
+  const Function *F = dyn_cast(GA.getAliasee());
+  

[PATCH] D150221: Add option -fkeep-persistent-storage-variables to emit all variables that have a persistent storage duration

[Zheng Qian via Phabricator via cfe-commits]

qianzhen updated this revision to Diff 540060.
qianzhen added a comment.

Add two more test cases


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150221

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/keep-persistent-storage-variables.cpp
  clang/test/Driver/fkeep-persistent-storage-variables.c

Index: clang/test/Driver/fkeep-persistent-storage-variables.c
===
--- /dev/null
+++ clang/test/Driver/fkeep-persistent-storage-variables.c
@@ -0,0 +1,5 @@
+// RUN: %clang -fkeep-persistent-storage-variables -c %s -### 2>&1 | FileCheck %s
+// RUN: %clang -fkeep-persistent-storage-variables -fno-keep-persistent-storage-variables -c %s -### 2>&1 | FileCheck %s -check-prefix=CHECK-NOKEEP
+
+// CHECK: "-fkeep-persistent-storage-variables"
+// CHECK-NOKEEP-NOT: "-fkeep-persistent-storage-variables"
Index: clang/test/CodeGen/keep-persistent-storage-variables.cpp
===
--- /dev/null
+++ clang/test/CodeGen/keep-persistent-storage-variables.cpp
@@ -0,0 +1,53 @@
+// RUN: %clang_cc1 -fkeep-persistent-storage-variables -emit-llvm %s -o - -triple=x86_64-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 -fkeep-persistent-storage-variables -emit-llvm %s -o - -triple=powerpc64-ibm-aix-xcoff | FileCheck %s
+
+// CHECK: @_ZL2g1 = internal global i32 0, align 4
+// CHECK: @_ZL2g2 = internal global i32 1, align 4
+// CHECK: @tl1 = thread_local global i32 0, align 4
+// CHECK: @tl2 = thread_local global i32 3, align 4
+// CHECK: @_ZL3tl3 = internal thread_local global i32 0, align 4
+// CHECK: @_ZL3tl4 = internal thread_local global i32 4, align 4
+// CHECK: @g5 = global i32 0, align 4
+// CHECK: @g6 = global i32 6, align 4
+// CHECK: @_ZZ5test3vE2s3 = internal global i32 0, align 4
+// CHECK: @_ZN12_GLOBAL__N_12s4E = internal global i32 42, align 4
+// CHECK: @_ZZ5test5vE3tl5 = internal thread_local global i32 1, align 4
+// CHECK: @_ZN2ST2s6E = global i32 7, align 4
+// CHECK: @_Z2v7 = internal global %union.anon zeroinitializer, align 4
+// CHECK: @_ZDC2v8E = global %struct.ST8 zeroinitializer, align 4
+// CHECK: @llvm{{(\.compiler)?}}.used = appending global [14 x ptr] [ptr @_ZL2g1, ptr @_ZL2g2, ptr @tl1, ptr @tl2, ptr @_ZL3tl3, ptr @_ZL3tl4, ptr @g5, ptr @g6, ptr @_ZZ5test3vE2s3, ptr @_ZN12_GLOBAL__N_12s4E, ptr @_ZZ5test5vE3tl5, ptr @_ZN2ST2s6E, ptr @_Z2v7, ptr @_ZDC2v8E], section "llvm.metadata"
+
+static int g1;
+static int g2 = 1;
+__thread int tl1;
+__thread int tl2 = 3;
+static __thread int tl3;
+static __thread int tl4 = 4;
+int g5;
+int g6 = 6;
+
+int test3() {
+  static int s3 = 0;
+  ++s3;
+  return s3;
+}
+
+namespace {
+  int s4 = 42;
+}
+
+int test5() {
+  static __thread int tl5 = 1;
+  ++tl5;
+  return tl5;
+}
+
+struct ST {
+  static int s6;
+};
+int ST::s6 = 7;
+
+static union { int v7; };
+
+struct ST8 { int v8; };
+auto [v8] = ST8{0};
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7294,6 +7294,8 @@
 
   Args.addOptInFlag(CmdArgs, options::OPT_fkeep_static_consts,
 options::OPT_fno_keep_static_consts);
+  Args.addOptInFlag(CmdArgs, options::OPT_fkeep_persistent_storage_variables,
+options::OPT_fno_keep_persistent_storage_variables);
   Args.addOptInFlag(CmdArgs, options::OPT_fcomplete_member_pointers,
 options::OPT_fno_complete_member_pointers);
   Args.addOptOutFlag(CmdArgs, options::OPT_fcxx_static_destructors,
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -2198,12 +2198,14 @@
   if (D && D->hasAttr())
 addUsedOrCompilerUsedGlobal(GV);
 
-  if (CodeGenOpts.KeepStaticConsts && D && isa(D)) {
-const auto *VD = cast(D);
-if (VD->getType().isConstQualified() &&
-VD->getStorageDuration() == SD_Static)
-  addUsedOrCompilerUsedGlobal(GV);
-  }
+  if (const auto *VD = dyn_cast_if_present(D);
+  VD &&
+  ((CodeGenOpts.KeepPersistentStorageVariables &&
+(VD->getStorageDuration() == SD_Static ||
+ VD->getStorageDuration() == SD_Thread)) ||
+   (CodeGenOpts.KeepStaticConsts && VD->getStorageDuration() == SD_Static &&
+VD->getType().isConstQualified(
+addUsedOrCompilerUsedGlobal(GV);
 }
 
 bool CodeGenModule::GetCPUAndFeaturesAttributes(GlobalDecl GD,
@@ -3084,12 +3086,14 @@
   if (LangOpts.EmitAllDecls)
 return true;
 
-  if (CodeGenOpts.KeepStaticConsts) {
-const auto *VD = dyn_cast(Global);
-if (VD && 

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

[Christian Walther via Phabricator via cfe-commits]

cwalther added a comment.

Okay. No, I don’t have write access (as far as I know) and would appreciate if 
you (or any other maintainer) could land the patch, with any commit message you 
deem appropriate.


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

https://reviews.llvm.org/D154357

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

[PATCH] D155195: [include-cleaner] Avoid a caching issue when running --edit mode on multiple files.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155195

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

[PATCH] D154688: [clang] Show verify prefix in error messages

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

When passing a different prefix via `-verify=foo`, the error messages now say 
"error: 'foo-error' diagnostics seen but not expected", etc.

I'm often working in test files where two different prefixes are used and I'm 
always confused about which one of the two the error messages are talking about.


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

https://reviews.llvm.org/D154688

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

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D154357#4496247 , @cwalther wrote:

> That summary wasn’t intended to be a commit message at all, but a description 
> of the problem and start of the discussion. My commit message was part of the 
> uploaded patch (since I generated it using `git show`) and said
>
>> [Driver] Recognize powerpc-*-unknown-eabi as a bare-metal toolchain
>>
>> Calling GCC as the linker as the Generic_ELF toolchain does is
>> inappropriate for an all-LLVM embedded toolchain.
>
> but it looks like Phabricator doesn’t show that anywhere.
>
> I haven’t user Phabricator before, so to make sure I am understanding you 
> correctly: You want me to edit the “summary” on this page, and then when you 
> commit the patch, it will automatically use that as the commit message? That 
> still seems much too verbose for a commit message to me even after removing 
> that last paragraph, but if you prefer that to the terse message above, 
> that’s fine with me.

Yes, edit the summary. You have done that. Thanks!

If you have write access to the llvm-project repository, you may adjust commit 
message before committing.
If you don't and ask someone else to land the patch for you, they may use `arc 
diff D154357` to fetch the patch, whose commit message will contain the 
Differential summary.


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

https://reviews.llvm.org/D154357

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

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

[Christian Walther via Phabricator via cfe-commits]

cwalther added a comment.

I have gone ahead and removed the last paragraph of the summary, assuming that 
was what was requested. Happy to make other edits.


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

https://reviews.llvm.org/D154357

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