[clang] e604f88 - [X86][test] Change some CodeGen tests to use %clang_cc1

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2022-11-03T22:54:44-07:00
New Revision: e604f88304e183d3ce46cea5ba6bfba2fe9fba36

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

LOG: [X86][test] Change some CodeGen tests to use %clang_cc1

Added: 


Modified: 
clang/test/CodeGen/X86/indirect-branch-cs-prefix.c
clang/test/CodeGen/X86/mmx-inline-asm.c
clang/test/CodeGen/X86/mmx-shift-with-immediate.c
clang/test/CodeGen/X86/x86-cf-protection.c

Removed: 




diff  --git a/clang/test/CodeGen/X86/indirect-branch-cs-prefix.c 
b/clang/test/CodeGen/X86/indirect-branch-cs-prefix.c
index 369db26677b4..67d2a69bc246 100644
--- a/clang/test/CodeGen/X86/indirect-branch-cs-prefix.c
+++ b/clang/test/CodeGen/X86/indirect-branch-cs-prefix.c
@@ -1,4 +1,4 @@
-// RUN: %clang -target i386-unknown-unknown -o - -emit-llvm -S 
-mindirect-branch-cs-prefix %s | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -triple i386 -o - -mindirect-branch-cs-prefix %s 
| FileCheck %s
 
 // CHECK: !{i32 4, !"indirect_branch_cs_prefix", i32 1}
 void foo() {}

diff  --git a/clang/test/CodeGen/X86/mmx-inline-asm.c 
b/clang/test/CodeGen/X86/mmx-inline-asm.c
index 635e2a6b71ef..19c24a3a91e1 100644
--- a/clang/test/CodeGen/X86/mmx-inline-asm.c
+++ b/clang/test/CodeGen/X86/mmx-inline-asm.c
@@ -1,5 +1,4 @@
-// RUN: %clang -mmmx -target i386-unknown-unknown -emit-llvm -S %s -o - | 
FileCheck %s
-// 
+// RUN: %clang_cc1 -emit-llvm -triple i386 -target-feature +mmx %s -o - | 
FileCheck %s
 #include 
 
 // CHECK: { x86_mmx, x86_mmx, x86_mmx, x86_mmx, x86_mmx, x86_mmx, x86_mmx }

diff  --git a/clang/test/CodeGen/X86/mmx-shift-with-immediate.c 
b/clang/test/CodeGen/X86/mmx-shift-with-immediate.c
index ecd1881c4875..83be6b5517c0 100644
--- a/clang/test/CodeGen/X86/mmx-shift-with-immediate.c
+++ b/clang/test/CodeGen/X86/mmx-shift-with-immediate.c
@@ -1,4 +1,4 @@
-// RUN: %clang -mmmx -target i386-unknown-unknown -emit-llvm -S %s -o - | 
FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -triple i386 -target-feature +mmx %s -o - | 
FileCheck %s
 #include 
 
 void shift(__m64 a, __m64 b, int c) {

diff  --git a/clang/test/CodeGen/X86/x86-cf-protection.c 
b/clang/test/CodeGen/X86/x86-cf-protection.c
index 9f0cafc2eb45..359bad714493 100644
--- a/clang/test/CodeGen/X86/x86-cf-protection.c
+++ b/clang/test/CodeGen/X86/x86-cf-protection.c
@@ -1,9 +1,9 @@
-// RUN: %clang -target i386-unknown-unknown -x c -E -dM -o - 
-fcf-protection=return %s | FileCheck %s --check-prefix=RETURN
-// RUN: %clang -target i386-unknown-unknown -x c -E -dM -o - 
-fcf-protection=branch %s | FileCheck %s --check-prefix=BRANCH
-// RUN: %clang -target i386-unknown-unknown -x c -E -dM -o - 
-fcf-protection=full %s   | FileCheck %s --check-prefix=FULL
-// RUN: %clang -target i386-unknown-unknown -o - -emit-llvm -S 
-fcf-protection=branch -mibt-seal -flto %s | FileCheck %s 
--check-prefixes=CFPROT,IBTSEAL
-// RUN: %clang -target i386-unknown-unknown -o - -emit-llvm -S 
-fcf-protection=branch -flto %s | FileCheck %s --check-prefixes=CFPROT,NOIBTSEAL
-// RUN: %clang -target i386-unknown-unknown -o - -emit-llvm -S 
-fcf-protection=branch -mibt-seal %s | FileCheck %s 
--check-prefixes=CFPROT,NOIBTSEAL
+// RUN: %clang_cc1 -E -triple i386 -dM -o - -fcf-protection=return %s | 
FileCheck %s --check-prefix=RETURN
+// RUN: %clang_cc1 -E -triple i386 -dM -o - -fcf-protection=branch %s | 
FileCheck %s --check-prefix=BRANCH
+// RUN: %clang_cc1 -E -triple i386 -dM -o - -fcf-protection=full %s   | 
FileCheck %s --check-prefix=FULL
+// RUN: %clang_cc1 -emit-llvm -triple i386 -o - -fcf-protection=branch 
-mibt-seal -flto %s | FileCheck %s --check-prefixes=CFPROT,IBTSEAL
+// RUN: %clang_cc1 -emit-llvm -triple i386 -o - -fcf-protection=branch -flto 
%s | FileCheck %s --check-prefixes=CFPROT,NOIBTSEAL
+// RUN: %clang_cc1 -emit-llvm -triple i386 -o - -fcf-protection=branch 
-mibt-seal %s | FileCheck %s --check-prefixes=CFPROT,NOIBTSEAL
 // RUN: not %clang_cc1 -emit-llvm-only -triple i386 -target-cpu pentium-mmx 
-fcf-protection=branch %s 2>&1 | FileCheck %s --check-prefix=NOCFPROT
 
 // RETURN: #define __CET__ 2



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

[clang] 3cbf904 - [X86][test] Add -fcf-protection test for pre-pentiumpro

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2022-11-03T22:21:44-07:00
New Revision: 3cbf90468aecce960887e680f813cbb1209b337f

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

LOG: [X86][test] Add -fcf-protection test for pre-pentiumpro

For #58737

Added: 


Modified: 
clang/test/CodeGen/X86/x86-cf-protection.c

Removed: 




diff  --git a/clang/test/CodeGen/X86/x86-cf-protection.c 
b/clang/test/CodeGen/X86/x86-cf-protection.c
index de6906ec51812..9f0cafc2eb456 100644
--- a/clang/test/CodeGen/X86/x86-cf-protection.c
+++ b/clang/test/CodeGen/X86/x86-cf-protection.c
@@ -4,6 +4,7 @@
 // RUN: %clang -target i386-unknown-unknown -o - -emit-llvm -S 
-fcf-protection=branch -mibt-seal -flto %s | FileCheck %s 
--check-prefixes=CFPROT,IBTSEAL
 // RUN: %clang -target i386-unknown-unknown -o - -emit-llvm -S 
-fcf-protection=branch -flto %s | FileCheck %s --check-prefixes=CFPROT,NOIBTSEAL
 // RUN: %clang -target i386-unknown-unknown -o - -emit-llvm -S 
-fcf-protection=branch -mibt-seal %s | FileCheck %s 
--check-prefixes=CFPROT,NOIBTSEAL
+// RUN: not %clang_cc1 -emit-llvm-only -triple i386 -target-cpu pentium-mmx 
-fcf-protection=branch %s 2>&1 | FileCheck %s --check-prefix=NOCFPROT
 
 // RETURN: #define __CET__ 2
 // BRANCH: #define __CET__ 1
@@ -11,4 +12,7 @@
 // CFPROT: !{i32 8, !"cf-protection-branch", i32 1}
 // IBTSEAL: !{i32 8, !"ibt-seal", i32 1}
 // NOIBTSEAL-NOT: "ibt-seal", i32 1
+
+// NOCFPROT: error: option 'cf-protection=branch' cannot be specified on this 
target
+
 void foo() {}



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

[PATCH] D136751: [clang][Interp] This pointers are writable in constructors

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

tbaeder added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136751

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

[PATCH] D136694: [clang][Interp] Check that constructor calls initialize all record fields

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

tbaeder added a comment.

Ping


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

https://reviews.llvm.org/D136694

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

[PATCH] D136457: [clang][Interp] Fix discarding non-primitive function call return values

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

tbaeder added a comment.

Ping


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

https://reviews.llvm.org/D136457

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

[PATCH] D135750: [clang][Interp] Track initialization state of local variables

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

tbaeder added a comment.

Ping


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

https://reviews.llvm.org/D135750

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

[PATCH] D134859: [clang][Interp] Implement basic support for floating point values

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

tbaeder added a comment.

Ping


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

https://reviews.llvm.org/D134859

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

[PATCH] D137386: [clang][Interp] Reject invalid declarations and expressions

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

tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, erichkeane, tahonermann, shafik.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

I'm not really sure if this is the right thing to do. These expressions and 
declarations have been marked as containing errors before, so they can't work 
properly during constant evaluation anyway. But I don't see them being rejected 
in the current constant interpreter.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137386

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp


Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -679,12 +679,18 @@
 }
 
 template  bool ByteCodeExprGen::discard(const Expr *E) 
{
+  if (E->containsErrors())
+return false;
+
   OptionScope Scope(this, /*NewDiscardResult=*/true);
   return this->Visit(E);
 }
 
 template 
 bool ByteCodeExprGen::visit(const Expr *E) {
+  if (E->containsErrors())
+return false;
+
   OptionScope Scope(this, /*NewDiscardResult=*/false);
   return this->Visit(E);
 }
@@ -1255,8 +1261,10 @@
 /// We need to evaluate the initializer and return its value.
 template 
 bool ByteCodeExprGen::visitDecl(const VarDecl *VD) {
-  Optional VarT = classify(VD->getType());
+  if (VD->isInvalidDecl())
+return false;
 
+  Optional VarT = classify(VD->getType());
   // Create and initialize the variable.
   if (!this->visitVarDecl(VD))
 return false;


Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -679,12 +679,18 @@
 }
 
 template  bool ByteCodeExprGen::discard(const Expr *E) {
+  if (E->containsErrors())
+return false;
+
   OptionScope Scope(this, /*NewDiscardResult=*/true);
   return this->Visit(E);
 }
 
 template 
 bool ByteCodeExprGen::visit(const Expr *E) {
+  if (E->containsErrors())
+return false;
+
   OptionScope Scope(this, /*NewDiscardResult=*/false);
   return this->Visit(E);
 }
@@ -1255,8 +1261,10 @@
 /// We need to evaluate the initializer and return its value.
 template 
 bool ByteCodeExprGen::visitDecl(const VarDecl *VD) {
-  Optional VarT = classify(VD->getType());
+  if (VD->isInvalidDecl())
+return false;
 
+  Optional VarT = classify(VD->getType());
   // Create and initialize the variable.
   if (!this->visitVarDecl(VD))
 return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136815: [clang][Interp] Unify visiting variable declarations

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

tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.h:282
+  bool isGlobalDecl(const VarDecl *VD) const {
+return !VD->hasLocalStorage() || VD->isConstexpr();
+  }

shafik wrote:
> tbaeder wrote:
> > shafik wrote:
> > > tbaeder wrote:
> > > > shafik wrote:
> > > > > Why not `hasGlobalStorage()`?
> > > > > 
> > > > > Also what is the logic behind `isConstexpr()` check?
> > > > Didn't know `isGlobalStorage()` existed ;)
> > > > 
> > > > Constexpr local variables can be handled like global ones, can't they? 
> > > > That was the logic behind it, nothing else. We can save ourselves the 
> > > > hassle of local variables in that case.
> > > I think I am missing a level of logic here. I don't think of constant 
> > > expressions as needing storage nor do I think of them as global variables.
> > > 
> > > So can you take a step back and explain how this fits in the bigger 
> > > picture?
> > They don't necessarily need storage in the final executable but we create 
> > global/local variables for them for bookkeeping, e.g. we need to be able to 
> > take their address, track the value, etc.
> Ok, will this work for recursive `constexpr` functions with local variables?
local `constexpr` variables still have to be initialized and are immutable, so 
that doesn't make a difference for recursive functions since none of the 
recursive calls can change the variable value. I did a small local test but 
since the variable can't be modified, it's not very interesting.


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

https://reviews.llvm.org/D136815

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

[PATCH] D137317: [X86][CET] Add Diags for targets pre to i686 for `-fcf-protection`

[Phoebe Wang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG525167829727: [X86][CET] Add Diags for targets pre to i686 
for `-fcf-protection` (authored by pengfei).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137317

Files:
  clang/lib/Basic/Targets/X86.h


Index: clang/lib/Basic/Targets/X86.h
===
--- clang/lib/Basic/Targets/X86.h
+++ clang/lib/Basic/Targets/X86.h
@@ -241,12 +241,16 @@
 
   bool
   checkCFProtectionReturnSupported(DiagnosticsEngine ) const override {
-return true;
+if (CPU == llvm::X86::CK_None || CPU >= llvm::X86::CK_PentiumPro)
+  return true;
+return TargetInfo::checkCFProtectionReturnSupported(Diags);
   };
 
   bool
   checkCFProtectionBranchSupported(DiagnosticsEngine ) const override {
-return true;
+if (CPU == llvm::X86::CK_None || CPU >= llvm::X86::CK_PentiumPro)
+  return true;
+return TargetInfo::checkCFProtectionBranchSupported(Diags);
   };
 
   virtual bool validateOperandSize(const llvm::StringMap ,


Index: clang/lib/Basic/Targets/X86.h
===
--- clang/lib/Basic/Targets/X86.h
+++ clang/lib/Basic/Targets/X86.h
@@ -241,12 +241,16 @@
 
   bool
   checkCFProtectionReturnSupported(DiagnosticsEngine ) const override {
-return true;
+if (CPU == llvm::X86::CK_None || CPU >= llvm::X86::CK_PentiumPro)
+  return true;
+return TargetInfo::checkCFProtectionReturnSupported(Diags);
   };
 
   bool
   checkCFProtectionBranchSupported(DiagnosticsEngine ) const override {
-return true;
+if (CPU == llvm::X86::CK_None || CPU >= llvm::X86::CK_PentiumPro)
+  return true;
+return TargetInfo::checkCFProtectionBranchSupported(Diags);
   };
 
   virtual bool validateOperandSize(const llvm::StringMap ,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 5251678 - [X86][CET] Add Diags for targets pre to i686 for `-fcf-protection`

[Phoebe Wang via cfe-commits]

Author: Phoebe Wang
Date: 2022-11-04T12:38:29+08:00
New Revision: 52516782972730ff065a34123a9d8876da08c254

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

LOG: [X86][CET] Add Diags for targets pre to i686 for `-fcf-protection`

Intel Control-flow Enforcement Technology (CET) provides new instructions 
`endbr32/64` for the indirect branch control. They are NOPs on i686 and new 
targets. We need to check for that in case it crashes on older targets.

Fixes #58737

Reviewed By: nickdesaulniers

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

Added: 


Modified: 
clang/lib/Basic/Targets/X86.h

Removed: 




diff  --git a/clang/lib/Basic/Targets/X86.h b/clang/lib/Basic/Targets/X86.h
index 71ab94601858..d4e6097f152f 100644
--- a/clang/lib/Basic/Targets/X86.h
+++ b/clang/lib/Basic/Targets/X86.h
@@ -241,12 +241,16 @@ class LLVM_LIBRARY_VISIBILITY X86TargetInfo : public 
TargetInfo {
 
   bool
   checkCFProtectionReturnSupported(DiagnosticsEngine ) const override {
-return true;
+if (CPU == llvm::X86::CK_None || CPU >= llvm::X86::CK_PentiumPro)
+  return true;
+return TargetInfo::checkCFProtectionReturnSupported(Diags);
   };
 
   bool
   checkCFProtectionBranchSupported(DiagnosticsEngine ) const override {
-return true;
+if (CPU == llvm::X86::CK_None || CPU >= llvm::X86::CK_PentiumPro)
+  return true;
+return TargetInfo::checkCFProtectionBranchSupported(Diags);
   };
 
   virtual bool validateOperandSize(const llvm::StringMap ,



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

[PATCH] D137381: [clang][compiler-rt] Exception escape out of an non-unwinding function is an undefined behaviour

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In your example, `clang++ a.cc; ./a.out` gives a libstdc++ error:

  terminate called after throwing an instance of 'int'

libc++'s is similar.

footgun is nounwind (due to the GNU pure attribute), so Clang uses `call` 
instead of `invoke` and the function call does not get a call site entry in 
`.gcc_except_table`.
In `_Unwind_RaiseException` called by `__cxa_throw`, the missing exception 
handler causes `__terminate`.

`g++ a.cc; ./a.out` succeeds, likely because its `footgun` call is caught by 
`catch (...)`.

The patch doesn't implement the runtime correctly (I get a linker error) so I 
cannot try it out. How expensive if your instrumentation?
Does it work with `-fno-exceptions` intermediate functions? (Unwinding through 
such functions should fail as well). Note that this check can be done 
today,without any instrumentation: just use `-fno-asynchronous-unwind-tables` 
(for some targets which default to async unwind tables (aarch64,x86,etc)).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137381

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

[PATCH] D137317: [X86][CET] Add Diags for targets pre to i686 for `-fcf-protection`

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added a comment.

Thanks @nickdesaulniers and @craig.topper!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137317

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

[PATCH] D137058: [Driver] [Modules] Support -fsave-std-c++-module-file (1/2)

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

> so I can appreciate the -fsave-std-c++-module-file name here, but it does 
> sound a bit verbose?

My thought is the option is mainly used by build systems so that the users 
wouldn't type it frequently. In this case, I feel it might not be bad to make 
it a little bit redundant. Someone's redundancy is another's readability : )

> Hmm, I don't mind that too much (& as you say, '-fobject-only' - though that 
> flag is maybe too vague?) - but it does mean we'd need a separate flag to 
> name the .pcm output file, because that flag ('-fmodule-only') wouldn't be 
> present in all cases where the pcm is generated, only when it's 
> pcm-but-no-object. Maybe less "exclusionary" flag names and more explicit 
> (like '-fbuild-the-binary-module' and '-fbuild-the-object'). I guess most C++ 
> GCC options ( https://gcc.gnu.org/onlinedocs/gcc/Option-Summary.html ) don't 
> include "C++" in them, but the modules ones mostly include "module".

If this paragraph only talks about the GCC?

> We could get out in front, planning for the world in which BMIs are really 
> only the interface (& whatever else we want to carry for optimizing /use/ of 
> that interface, but not complete enough to be usable for generating the 
> modules object) and use -fmodule-interface[=] (skipping the 'binary' part) 
> and -fmodule-implementation or -fmodule-object (which could go either way - 
> default on or off, but provide -fno-module-object to do the "generate PCM 
> only")?

For clang, I would like to add `c++` in the option name since users are really 
easy to  get confused with the existing clang modules. I've met many such 
cases. And for the proposal it self, I feel it is complex and hard to 
understand.

>> GCC has "-fmodule-only" for which I have a patch (also unpublished) that 
>> aliases that to --precompile in the driver.

This idea sounds good to me. But I would like to suggest to change the name to 
`-fc++-module-interface-only`. Due to the same reason above. The name 
`-fmodule-only` is not clear especially in the context of clang now.

> Only other minor thing might be singular V plural - some of Clang's flags are 
> -fmodules and others are -fmodule - any sense of what's likely to work 
> better? (be nice to unify/standardize on one or the other, I can see having 
> variation there might make for frustrating usability trying to remember which 
> is which)

Yeah, sometimes it is frustrating. But I think we can't unify the existing 
options. It'd cause many breaking changes. And for `module` and `modules`, I 
prefer `module` a little more in practice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137058

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

[PATCH] D136919: [X86][RFC] Change mangle name of __bf16 from u6__bf16 to DF16b

[Phoebe Wang via Phabricator via cfe-commits]

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

In D136919#3906159 , @rjmccall wrote:

> In D136919#3906133 , @rjmccall 
> wrote:
>
>> We talked about this on the Itanium list, and as currently specified, it is 
>> absolutely not correct for `__bf16` to have the same mangling as 
>> `std::bfloat16_t`, because `__bf16` does not have the correct semantics for 
>> `std::bfloat16_t` and must be a distinct type.  If GCC changed `__bf16` to 
>> use the new mangling without also updating the semantics, it is a bug.
>>
>> That discussion was here: https://github.com/itanium-cxx-abi/cxx-abi/pull/147
>>
>> If we want to implement `std::bfloat16_t` in Clang, we need to make it a 
>> normal arithmetic type, and in practice it needs to guarantee 
>> excess-precision arithmetic, as I discussed on that thread.  Coincidentally, 
>> we did recently implement excess-precision arithmetic in Clang for 
>> `_Float16`.
>
> Jakub Jelinek has clarified that GCC did indeed change the semantics of 
> `__bf16` on i386 and x86_64 to be a proper extended floating point type.
>
> We could change the mangling to match GCC, but I think it would be 
> inappropriate to do that without also matching the semantics change.  Since 
> the mangling change is trivial to land, I think the semantics change should 
> happen first.

Thanks @rjmccall for pointing to the Itanium list. I'm clear about what to do 
now. Yes, fully agreed, we should match the semantics at the same time.

In D136919#3905891 , @tra wrote:

> NVPTX has no current uses of this, so if the mangling needs to change, now 
> would be the good time.

Thanks @tra. Per to @rjmccall's information. The mangling should be aligned 
with its semantics. It's fine to use `u6__bf16` if a target doesn't want to 
support arithmetic operations.

And thanks @RKSimon, @tschuett and @aaron.ballman for your kind inputs!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136919

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

[PATCH] D137259: [clang][modules][deps] WIP: In-memory module transfer

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 473104.
jansvoboda11 added a comment.

Kinda rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137259

Files:
  clang/include/clang/Frontend/CompilerInstance.h
  clang/include/clang/Lex/PreprocessorOptions.h
  clang/include/clang/Lex/Token.h
  clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/Serialization/GeneratePCH.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
  clang/test/ClangScanDeps/modules-basic.c

Index: clang/test/ClangScanDeps/modules-basic.c
===
--- /dev/null
+++ clang/test/ClangScanDeps/modules-basic.c
@@ -0,0 +1,104 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+//--- include/module.modulemap
+module m { header "m.h"}
+module x { header "x.h" }
+//--- include/m.h
+#include "x.h"
+#define FOO
+//--- include/x.h
+// empty
+//--- include/t.h
+
+//--- tu.c
+#include "m.h"
+#ifdef FOO
+#include "t.h"
+#endif
+
+//--- cdb.json.template
+[{
+  "file": "DIR/tu.c",
+  "directory": "DIR",
+  "command": "clang -fsyntax-only DIR/tu.c -fmodules -fmodules-cache-path=DIR/cache -fimplicit-module-maps -I DIR/include"
+}]
+
+// RUN: sed "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json
+
+// RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full > %t/result.json
+// RUN: cat %t/result.json | sed 's:\?:/:g' | FileCheck %s -DPREFIX=%/t
+
+// CHECK: {
+// CHECK-NEXT:   "modules": [
+// CHECK-NEXT: {
+// CHECK-NEXT:   clang-module-deps": [
+// CHECK-NEXT:{
+// CHECK-NEXT:  "context-hash": "[[X_HASH:.*]]",
+// CHECK-NEXT:  "module-name": "x"
+// CHECK-NEXT:}
+// CHECK-NEXT:   ],
+// CHECK-NEXT:   "clang-modulemap-file": "[[PREFIX]]/include/module.modulemap",
+// CHECK-NEXT:   "command-line": [
+// CHECK-NEXT: "-cc1",
+// CHECK:  "-o",
+// CHECK-NEXT: "[[PREFIX]]/cache/{{.*}}/m-{{.*}}.pcm",
+// CHECK:  "-emit-module",
+// CHECK:  "[[PREFIX]]/include/module.modulemap",
+// CHECK:  "-fmodules",
+// CHECK:  "-fmodule-name=m",
+// CHECK:],
+// CHECK-NEXT:   "context-hash": "[[M_HASH:.*]]",
+// CHECK-NEXT:   "file-deps": [
+// CHECK-NEXT: "[[PREFIX]]/include/m.h",
+// CHECK-NEXT: "[[PREFIX]]/include/module.modulemap"
+// CHECK-NEXT:   ],
+// CHECK-NEXT:   "name": "m"
+// CHECK-NEXT: },
+// CHECK-NEXT: {
+// CHECK-NEXT:   "clang-module-deps": [],
+// CHECK-NEXT:   "clang-modulemap-file": "[[PREFIX]]/include/module.modulemap",
+// CHECK-NEXT:   "command-line": [
+// CHECK-NEXT: "-cc1",
+// CHECK:  "-o",
+// CHECK-NEXT: "[[PREFIX]]/cache/{{.*}}/x-{{.*}}.pcm",
+// CHECK:  "-emit-module",
+// CHECK:  "[[PREFIX]]/include/module.modulemap",
+// CHECK:  "-fmodules",
+// CHECK:  "-fmodule-name=x",
+// CHECK:],
+// CHECK-NEXT:   "context-hash": "[[X_HASH]]",
+// CHECK-NEXT:"file-deps": [
+// CHECK-NEXT:  "[[PREFIX]]/include/module.modulemap",
+// CHECK-NEXT:  "[[PREFIX]]/include/x.h"
+// CHECK-NEXT:],
+// CHECK-NEXT:   "name": "x"
+// CHECK-NEXT: }
+// CHECK-NEXT:   ],
+// CHECK-NEXT:   "translation-units": [
+// CHECK-NEXT: {
+// CHECK-NEXT:   "commands": [
+// CHECK-NEXT: {
+// CHECK-NEXT:   "clang-context-hash": "{{.*}}",
+// CHECK-NEXT:   "clang-module-deps": [
+// CHECK-NEXT: {
+// CHECK-NEXT:   "context-hash": "[[M_HASH]]",
+// CHECK-NEXT:   "module-name": "m"
+// CHECK-NEXT: }
+// CHECK-NEXT:   ],
+// CHECK-NEXT:   "command-line": [
+// CHECK-NEXT: "-cc1",
+// CHECK:  "-fmodule-map-file=[[PREFIX]]/include/module.modulemap",
+// CHECK:  "-fmodule-file=m=[[PREFIX]]/cache/{{.*}}/m-{{.*}}.pcm",
+// CHECK:],
+// CHECK-NEXT:   "executable": "clang",
+// CHECK-NEXT:   "file-deps": [
+// CHECK-NEXT: "[[PREFIX]]/tu.c",
+// CHECK-NEXT: "[[PREFIX]]/include/t.h"
+// CHECK-NEXT:   ],
+// CHECK-NEXT:   "input-file": "[[PREFIX]]/tu.c"
+// CHECK-NEXT: }
+// CHECK-NEXT:   ]
+// CHECK-NEXT: }
+// CHECK-NEXT:   ]
+// CHECK-NEXT: }
Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -19,24 +19,22 @@
 

[PATCH] D135937: [X86] Support -march=raptorlake, meteorlake

[Freddy, Ye 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 rGa806fc2767d7: [X86] Support -march=raptorlake, meteorlake 
(authored by FreddyYe).

Changed prior to commit:
  https://reviews.llvm.org/D135937?vs=472822=473100#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135937

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Basic/Targets/X86.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/attr-target-mv.c
  clang/test/CodeGen/target-builtin-noerror.c
  clang/test/Driver/x86-march.c
  clang/test/Misc/target-invalid-cpu-note.c
  clang/test/Preprocessor/predefined-arch-macros.c
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/Support/X86TargetParser.def
  llvm/include/llvm/Support/X86TargetParser.h
  llvm/lib/Support/Host.cpp
  llvm/lib/Support/X86TargetParser.cpp
  llvm/lib/Target/X86/X86.td
  llvm/test/CodeGen/X86/cpus-intel.ll

Index: llvm/test/CodeGen/X86/cpus-intel.ll
===
--- llvm/test/CodeGen/X86/cpus-intel.ll
+++ llvm/test/CodeGen/X86/cpus-intel.ll
@@ -17,6 +17,8 @@
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=yonah 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=prescott 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=lakemont 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=raptorlake 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=meteorlake 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=nocona 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=core2 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
@@ -52,6 +54,8 @@
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=tremont 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=knl 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=knm 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=raptorlake 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=meteorlake 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 
 define void @foo() {
   ret void
Index: llvm/lib/Target/X86/X86.td
===
--- llvm/lib/Target/X86/X86.td
+++ llvm/lib/Target/X86/X86.td
@@ -1502,6 +1502,10 @@
 ProcessorFeatures.SPRFeatures, ProcessorFeatures.SPRTuning>;
 def : ProcModel<"alderlake", AlderlakePModel,
 ProcessorFeatures.ADLFeatures, ProcessorFeatures.ADLTuning>;
+def : ProcModel<"raptorlake", AlderlakePModel,
+ProcessorFeatures.ADLFeatures, ProcessorFeatures.ADLTuning>;
+def : ProcModel<"meteorlake", AlderlakePModel,
+ProcessorFeatures.ADLFeatures, ProcessorFeatures.ADLTuning>;
 
 // AMD CPUs.
 
Index: llvm/lib/Support/X86TargetParser.cpp
===
--- llvm/lib/Support/X86TargetParser.cpp
+++ llvm/lib/Support/X86TargetParser.cpp
@@ -370,6 +370,10 @@
   { {"sapphirerapids"}, CK_SapphireRapids, FEATURE_AVX512BF16, FeaturesSapphireRapids },
   // Alderlake microarchitecture based processors.
   { {"alderlake"}, CK_Alderlake, FEATURE_AVX2, FeaturesAlderlake },
+  // Raptorlake microarchitecture based processors.
+  { {"raptorlake"}, CK_Raptorlake, FEATURE_AVX2, FeaturesAlderlake },
+  // Meteorlake microarchitecture based processors.
+  { {"meteorlake"}, CK_Meteorlake, FEATURE_AVX2, FeaturesAlderlake },
   // Knights Landing processor.
   { {"knl"}, CK_KNL, FEATURE_AVX512F, FeaturesKNL },
   // Knights Mill processor.
Index: llvm/lib/Support/Host.cpp
===
--- llvm/lib/Support/Host.cpp
+++ llvm/lib/Support/Host.cpp
@@ -816,6 +816,12 @@
 // Alderlake:
 case 0x97:
 case 0x9a:
+// Raptorlake:
+case 0xb7:
+// Meteorlake:
+case 0xb5:
+case 0xaa:
+case 0xac:
   CPU = "alderlake";
   *Type = X86::INTEL_COREI7;
   *Subtype = X86::INTEL_COREI7_ALDERLAKE;
Index: llvm/include/llvm/Support/X86TargetParser.h
===
--- 

[clang] a806fc2 - [X86] Support -march=raptorlake, meteorlake

[Freddy Ye via cfe-commits]

Author: Freddy Ye
Date: 2022-11-04T09:32:17+08:00
New Revision: a806fc2767d74f2d052647e272dd4339bd747bf0

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

LOG: [X86] Support -march=raptorlake, meteorlake

Reviewed By: pengfei, skan, MaskRay

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Basic/Targets/X86.cpp
clang/lib/CodeGen/CGBuiltin.cpp
clang/test/CodeGen/attr-target-mv.c
clang/test/CodeGen/target-builtin-noerror.c
clang/test/Driver/x86-march.c
clang/test/Misc/target-invalid-cpu-note.c
clang/test/Preprocessor/predefined-arch-macros.c
llvm/docs/ReleaseNotes.rst
llvm/include/llvm/Support/X86TargetParser.def
llvm/include/llvm/Support/X86TargetParser.h
llvm/lib/Support/Host.cpp
llvm/lib/Support/X86TargetParser.cpp
llvm/lib/Target/X86/X86.td
llvm/test/CodeGen/X86/cpus-intel.ll

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 73d7aff9b8910..ad1a00b4bbcc4 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -705,6 +705,7 @@ X86 Support in Clang
   * Support intrinsic of ``_mm(256)_cvtneobf16_ps``.
   * Support intrinsic of ``_mm(256)_cvtneoph_ps``.
   * Support intrinsic of ``_mm(256)_cvtneps_avx_pbh``.
+- ``-march=raptorlake`` and ``-march=meteorlake`` are now supported.
 
 WebAssembly Support in Clang
 

diff  --git a/clang/lib/Basic/Targets/X86.cpp b/clang/lib/Basic/Targets/X86.cpp
index 2d3f3d10c5716..a33a6f06c0182 100644
--- a/clang/lib/Basic/Targets/X86.cpp
+++ b/clang/lib/Basic/Targets/X86.cpp
@@ -524,6 +524,8 @@ void X86TargetInfo::getTargetDefines(const LangOptions 
,
   case CK_Tigerlake:
   case CK_SapphireRapids:
   case CK_Alderlake:
+  case CK_Raptorlake:
+  case CK_Meteorlake:
 // FIXME: Historically, we defined this legacy name, it would be nice to
 // remove it at some point. We've never exposed fine-grained names for
 // recent primary x86 CPUs, and we should keep it that way.
@@ -1194,6 +1196,7 @@ bool X86TargetInfo::validateCpuIs(StringRef FeatureStr) 
const {
 #define X86_VENDOR(ENUM, STRING) .Case(STRING, true)
 #define X86_CPU_TYPE_ALIAS(ENUM, ALIAS) .Case(ALIAS, true)
 #define X86_CPU_TYPE(ENUM, STR) .Case(STR, true)
+#define X86_CPU_SUBTYPE_ALIAS(ENUM, ALIAS) .Case(ALIAS, true)
 #define X86_CPU_SUBTYPE(ENUM, STR) .Case(STR, true)
 #include "llvm/Support/X86TargetParser.def"
   .Default(false);
@@ -1408,6 +1411,8 @@ Optional X86TargetInfo::getCPUCacheLineSize() 
const {
 case CK_Rocketlake:
 case CK_IcelakeServer:
 case CK_Alderlake:
+case CK_Raptorlake:
+case CK_Meteorlake:
 case CK_KNL:
 case CK_KNM:
 // K7

diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 8bb5626392bd3..b0da121340556 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -12935,6 +12935,8 @@ Value *CodeGenFunction::EmitX86CpuIs(StringRef CPUStr) {
   .Case(ALIAS, {1u, static_cast(llvm::X86::ENUM)})
 #define X86_CPU_TYPE(ENUM, STR)
\
   .Case(STR, {1u, static_cast(llvm::X86::ENUM)})
+#define X86_CPU_SUBTYPE_ALIAS(ENUM, ALIAS) 
\
+  .Case(ALIAS, {2u, static_cast(llvm::X86::ENUM)})
 #define X86_CPU_SUBTYPE(ENUM, STR) 
\
   .Case(STR, {2u, static_cast(llvm::X86::ENUM)})
 #include "llvm/Support/X86TargetParser.def"

diff  --git a/clang/test/CodeGen/attr-target-mv.c 
b/clang/test/CodeGen/attr-target-mv.c
index e5241a1bbe54e..581f18e10b081 100644
--- a/clang/test/CodeGen/attr-target-mv.c
+++ b/clang/test/CodeGen/attr-target-mv.c
@@ -15,6 +15,8 @@ int __attribute__((target("arch=sapphirerapids"))) foo(void) 
{return 10;}
 int __attribute__((target("arch=alderlake"))) foo(void) {return 11;}
 int __attribute__((target("arch=rocketlake"))) foo(void) {return 12;}
 int __attribute__((target("arch=core2"))) foo(void) {return 13;}
+int __attribute__((target("arch=raptorlake"))) foo(void) {return 14;}
+int __attribute__((target("arch=meteorlake"))) foo(void) {return 15;}
 int __attribute__((target("default"))) foo(void) { return 2; }
 
 int bar(void) {
@@ -149,6 +151,10 @@ void calls_pr50025c(void) { pr50025c(); }
 // LINUX: ret i32 12
 // LINUX: define{{.*}} i32 @foo.arch_core2()
 // LINUX: ret i32 13
+// LINUX: define{{.*}} i32 @foo.arch_raptorlake()
+// LINUX: ret i32 14
+// LINUX: define{{.*}} i32 @foo.arch_meteorlake()
+// LINUX: ret i32 15
 // LINUX: define{{.*}} i32 @foo()
 // LINUX: ret i32 2
 // LINUX: define{{.*}} i32 @bar()
@@ -180,6 +186,10 @@ void calls_pr50025c(void) { pr50025c(); }
 // WINDOWS: ret i32 12
 // WINDOWS: define 

[PATCH] D137379: -Wunsafe-buffer-usage: adding warnings for unsafe buffer accesses by array subscript operations

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Yes, so this is how you work with the abstraction introduced in D137348 
. It's very easy to cover more and more 
patterns incrementally.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137379

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

[PATCH] D136811: -Wunsafe-buffer-usage: WIP: RFC: NFC: User documentation.

[Jan Korous via Phabricator via cfe-commits]

jkorous added inline comments.



Comment at: clang/docs/SafeBuffers.rst:173
+  #pragma unsafe_buffer_usage begin
+
+Such pragmas not only enable incremental adoption with much smaller 
granularity,

aaron.ballman wrote:
> jkorous wrote:
> > aaron.ballman wrote:
> > > Have you considered allowing a short-hand form of the pragmas that are 
> > > scoped to the current block scope (or file scope if applied at global 
> > > scope)? e.g.,
> > > ```
> > > void func() {
> > >   {
> > > #pragma only_safe_buffers
> > > int stuff[10] = {};
> > > int x = stuff[1]; // Warning: you're a bad person who should feel bad
> > >   }
> > >   int array[10] = {};
> > >   int y = array[1]; // this_is_fine.png
> > > }
> > > ```
> > We discussed other options but generally prefer simple pragmas for their 
> > flexibility.
> > Our current thinking is that for readability purposes we won't allow 
> > nesting of pragmas and would have end of scope to be explicit.
> > While this might be more verbose it would be dead simple to reason about.
> > We discussed other options but generally prefer simple pragmas for their 
> > flexibility.
> 
> A single pragma with lexical scoping behaves like RAII, which is often a 
> replacement for more complex acquire/release paired operations and is a 
> pretty well-understood pattern in C++.
> 
> > Our current thinking is that for readability purposes we won't allow 
> > nesting of pragmas and would have end of scope to be explicit.
> > While this might be more verbose it would be dead simple to reason about.
> 
> True, it is easy to reason about. But not allowing nesting for readability 
> purposes removes some utility, especially for folks in C where macros are 
> much more common. e.g.,
> ```
> #define ONLY_SAFE_BUFFERS _Pragma("only_safe_buffers")
> 
> #define AWESOME_BUFFER_THING(ptr, size) ({ ONLY_SAFE_BUFFERS; (interesting 
> work goes here) })
> 
> void func(int *ptr, size_t size) {
>   ONLY_SAFE_BUFFERS;
>   ... interesting code lives here ...
>   AWESOME_BUFFER_THING(ptr, size); // Oops, this is nested.
>  .. more interesting code lives here ...
> }
> ```
> (You can hit the same situation with paired pragmas.) Because C doesn't have 
> constexpr or consteval support for functions or templates, function-like 
> macros are a pretty common interface folks use in practice and macro 
> expansion makes it somewhat hard to avoid potential surprising conflicts.
It is desirable to minimize the extent of code labeled as unsafe - if a single 
line of code is unsafe then it'd be suboptimal do label a whole scope as unsafe.
Since scopes affect objects lifetime in C++ we won't be able to generally 
recommend adding a scope around the unsafe line either as that might change 
behavior of the code.
In this sense associating the pragmas' semantics with scopes reduces their 
flexibility. Is there any benefit of doing that which would outweigh this?

Separately from that - in theory the users could just end the scope before the 
macro:
```
  ONLY_SAFE_BUFFERS_BEGIN;
  ... interesting code lives here ...
  ONLY_SAFE_BUFFERS_END;
  AWESOME_BUFFER_THING(ptr, size); // Oops, this is nested.
```

But point taken - there might be options for better ergonomics.



Comment at: clang/docs/SafeBuffers.rst:213
+
+The attribute is NOT warranted when the function has runtime protection against
+overflows, assuming hardened libc++, assuming that containers constructed

NoQ wrote:
> aaron.ballman wrote:
> > jkorous wrote:
> > > aaron.ballman wrote:
> > > > One thing I think is worth pointing out about these docs is that the 
> > > > first example effectively says "do add the attribute because the size 
> > > > passed in to the function could be wrong" and the second example says 
> > > > "don't add the attribute on the assumption that the container has the 
> > > > correct size information". The advice feels a bit conflicting to me 
> > > > because in one case we're assuming callers pass in incorrect values and 
> > > > in the other case we're assuming callers pass in correct values and the 
> > > > only distinction between them is a "container was used".  But a 
> > > > significant portion of our users don't use libc++ (they use libstdc++ 
> > > > or MS STL for example).
> > > > 
> > > > I think we should have more details on why the STL used at runtime 
> > > > doesn't matter, or if it still really does matter, we may want to 
> > > > reconsider the advice we give.
> > > > 
> > > > Also, we don't give similar advice for use of the pragmas. Should we? 
> > > > (Maybe split the advice as to when to use markings in general out into 
> > > > its own section and reference it from both the pragma and attribute 
> > > > sections?)
> > > > One thing I think is worth pointing out about these docs is that the 
> > > > first example effectively says "do add the attribute because the size 
> > > > passed in to the function could be wrong" and the second 

[PATCH] D137381: [clang][compiler-rt] Exception escape out of an non-unwinding function is an undefined behaviour

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

(also, this needs a bit more work around irgen, will look in a bit.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137381

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

[PATCH] D137381: [clang][compiler-rt] Exception escape out of an non-unwinding function is an undefined behaviour

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri created this revision.
lebedev.ri added reviewers: rjmccall, morehouse, aaron.ballman, dvyukov, 
MaskRay, vsk, Sanitizers.
lebedev.ri added a project: LLVM.
Herald added subscribers: Enna1, StephenFan, dberris.
Herald added a project: All.
lebedev.ri requested review of this revision.
Herald added projects: clang, Sanitizers.
Herald added a subscriber: cfe-commits.

I've stumbled into this in 
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=52989=label%3AProj-librawspeed
which manifested as an obscure leak, and originated from a seemingly simple 
refactoring:
https://github.com/darktable-org/rawspeed/commit/1fd09b9cffbddc65753eb523f7ba5528d35fe34d#diff-c832cc8366d36ca1165ecef7f4a256a2643ec09c4405a1238222a4529df619a1R172-R174

Reduced, this looked like:

  #include 
  
  std::vector handle() {
std::vector v(42, 42); // this somehow leaks
return v;
  }
  
  __attribute__((pure)) // double yikes
  std::vector footgun(int argc) {
std::vector v(24, 24);
if(argc != 42)
  throw int(0); // yikes
return v;
  }
  
  int main(int argc, char* argv[]) {
  try {
  auto v = handle();
  auto v2 = footgun(argc);
  } catch(...) {}
  return 0;
  }

https://godbolt.org/z/zdavdKnfa

Surprisingly, we did not handle it before.
I'm not yet sure if we can pretty-print the actual exception on compiler-rt 
side.

(I may need to update a couple tests still)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137381

Files:
  clang/docs/ReleaseNotes.rst
  clang/docs/UndefinedBehaviorSanitizer.rst
  clang/include/clang/Basic/Sanitizers.def
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGException.cpp
  clang/lib/CodeGen/CMakeLists.txt
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/CodeGenCXX/catch-exception-escape.cpp
  compiler-rt/lib/ubsan/ubsan_checks.inc
  compiler-rt/lib/ubsan/ubsan_handlers.cpp
  compiler-rt/lib/ubsan/ubsan_handlers.h
  compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
  compiler-rt/test/ubsan/TestCases/Misc/exception-escape.cpp

Index: compiler-rt/test/ubsan/TestCases/Misc/exception-escape.cpp
===
--- /dev/null
+++ compiler-rt/test/ubsan/TestCases/Misc/exception-escape.cpp
@@ -0,0 +1,33 @@
+// RUN: %clangxx -fsanitize=exception-escape %s -O3 -o %t
+// RUN: %run %t 2>&1 | FileCheck %s --implicit-check-not="error:" --check-prefixes=CHECK-ALL,CHECK-ONE
+// RUN: not %run %t a 2>&1 | FileCheck %s --implicit-check-not="error:" --check-prefixes=CHECK-ALL,CHECK-TWO
+// RUN: not %run %t a b 2>&1 | FileCheck %s --implicit-check-not="error:" --check-prefixes=CHECK-ALL,CHECK-THREE
+
+#include 
+
+void thrower() { throw 0; }
+
+void footgun(int x) noexcept {
+#line 100
+  if (x == 2)
+thrower();
+#line 200
+  if (x == 3)
+thrower();
+}
+
+// CHECK-ALL: TEST
+
+// CHECK-ONE-EMPTY:
+// CHECK-TWO: exception-escape.cpp:101:5: runtime error: exception escapes out of function that should not throw exception
+// CHECK-THREE: exception-escape.cpp:201:5: runtime error: exception escapes out of function that should not throw exception
+
+int main(int argc, char **argv) {
+  fprintf(stderr, "TEST\n");
+
+  try {
+footgun(argc);
+  } catch (...) {
+  }
+  return 0;
+}
Index: compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
===
--- compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
+++ compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
@@ -141,3 +141,4 @@
 HANDLER(nullability_return, "nullability-return")
 HANDLER(pointer_overflow, "pointer-overflow")
 HANDLER(cfi_check_fail, "cfi-check-fail")
+HANDLER(exception_escape, "exception-escape")
Index: compiler-rt/lib/ubsan/ubsan_handlers.h
===
--- compiler-rt/lib/ubsan/ubsan_handlers.h
+++ compiler-rt/lib/ubsan/ubsan_handlers.h
@@ -99,6 +99,13 @@
 /// \brief Handle reaching the end of a value-returning function.
 UNRECOVERABLE(missing_return, UnreachableData *Data)
 
+struct ExceptionEscapeData {
+  SourceLocation Loc;
+};
+
+/// \brief Handle exception escaping out of C++ `noexcept` function.
+UNRECOVERABLE(exception_escape, ExceptionEscapeData *Data, ValueHandle Exn)
+
 struct VLABoundData {
   SourceLocation Loc;
   const TypeDescriptor 
Index: compiler-rt/lib/ubsan/ubsan_handlers.cpp
===
--- compiler-rt/lib/ubsan/ubsan_handlers.cpp
+++ compiler-rt/lib/ubsan/ubsan_handlers.cpp
@@ -433,6 +433,17 @@
   Die();
 }
 
+void __ubsan::__ubsan_handle_exception_escape(ExceptionEscapeData *Data,
+  ValueHandle Exn) {
+  GET_REPORT_OPTIONS(true);
+  ErrorType ET = ErrorType::ExceptionEscape;
+  ScopedReport R(Opts, Data->Loc, ET);
+  Diag(Data->Loc, DL_Error, ET,
+   "exception escapes out of function that should not throw exception");

[PATCH] D136846: [Driver] Add -fsample-profile-use-profi

[Fangrui Song via Phabricator via cfe-commits]

MaskRay requested changes to this revision.
MaskRay added inline comments.
This revision now requires changes to proceed.



Comment at: clang/docs/UsersManual.rst:2246
+  blocks. Profi (profile inference) algorithm can infer block and edge counts
+  to fix them. For applying this, you can use an additional flag
+  ``-fsample-profile-use-profi`` to the command line.

This sentence is unnecessarily long. Just say `Enable it with 
-fsample-profile-use-profi.` (with correct backquotes)



Comment at: clang/include/clang/Driver/Options.td:1253
+Flags<[NoXarchOption, CC1Option]>, Group,
+HelpText<"Use profi to infer block and edge counts.">,
+DocBrief<[{Infer block and edge counts. If the profiles have errors or 
missing

Omit trailing `.` in HelpText.



Comment at: clang/test/Driver/pgo-sample-use-profi.c:1
+// Test if profi flat is enabled in frontend as user-facing feature.
+//

Optional: `///`

Make non-RUN non-CHECK comments stand out and help tools finding 
unused/incorrect prefixes.



Comment at: clang/test/Driver/pgo-sample-use-profi.c:2
+// Test if profi flat is enabled in frontend as user-facing feature.
+//
+// RUN: %clang -c -fsample-profile-use-profi 
-fprofile-sample-use=%S/../CodeGen/Inputs/pgo-sample.prof -### %s 2>&1 | 
FileCheck %s

`//` => ``


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136846

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

[PATCH] D137268: [clang][Headers] Do not define varargs macros for __need___va_list

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/Headers/stdarg.h:17
+#ifndef __GNUC_VA_LIST
+#define __GNUC_VA_LIST 1
+typedef __builtin_va_list __gnuc_va_list;

To match gcc stdarg.h, `#define __GNUC_VA_LIST`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137268

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

[PATCH] D137287: [Test] Fix CHECK typo.

[Paul Robinson via Phabricator via cfe-commits]

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

LGTM, but you'll want to be ready to jump on any bot failures.  That's just the 
nature of these kinds of changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137287

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

[PATCH] D131919: Move googletest to the third-party directory

[Paul Robinson via Phabricator via cfe-commits]

probinson accepted this revision.
probinson added a comment.

Yes, LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131919

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

[PATCH] D137379: -Wunsafe-buffer-usage: adding warnings for unsafe buffer accesses by array subscript operations

[Ziqing Luo via Phabricator via cfe-commits]

ziqingluo-90 created this revision.
ziqingluo-90 added reviewers: NoQ, t-rasmud, malavikasamak, jkorous, xazax.hun, 
aaron.ballman, gribozavr2.
Herald added a subscriber: rnkovacs.
Herald added a project: All.
ziqingluo-90 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Adding an unsafe `Gadget` to represent and search  (via matchers)  for unsafe 
buffer accesses through array subscript operations.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137379

Files:
  clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
  clang/lib/Analysis/UnsafeBufferUsage.cpp
  clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp

Index: clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
===
--- clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
@@ -6,3 +6,57 @@
   --p; // expected-warning{{unchecked operation on raw buffer in expression}}
   p--; // expected-warning{{unchecked operation on raw buffer in expression}}
 }
+
+void foo(...);
+
+void * bar(void);
+char * baz(void);
+
+void testArraySubscripts(int *p, int **pp) {
+  foo(p[0], // expected-warning{{unchecked operation on raw buffer in expression}}
+  pp[0][0], // expected-warning2{{unchecked operation on raw buffer in expression}}
+  0[0[pp]], // expected-warning2{{unchecked operation on raw buffer in expression}}
+  0[pp][0]  // expected-warning2{{unchecked operation on raw buffer in expression}}
+  );
+
+  if (p[3]) {   // expected-warning{{unchecked operation on raw buffer in expression}}
+void * q = p;
+
+foo(((int*)q)[10]); // expected-warning{{unchecked operation on raw buffer in expression}}
+  }
+
+  foo(((int*)bar())[3], // expected-warning{{unchecked operation on raw buffer in expression}}
+  3[(int*)bar()],   // expected-warning{{unchecked operation on raw buffer in expression}}
+  baz()[3], // expected-warning{{unchecked operation on raw buffer in expression}}
+  3[baz()]  // expected-warning{{unchecked operation on raw buffer in expression}}
+  );
+
+  int a[10], b[10][10];
+
+  // not to warn subscripts on arrays
+  foo(a[0], a[1],
+  0[a], 1[a],
+  b[3][4],
+  4[b][3],
+  4[3[b]]);
+}
+
+void testArraySubscriptsWithAuto(int *p, int **pp) {
+  int a[10];
+
+  auto ap1 = a;
+
+  foo(ap1[0]);  // expected-warning{{unchecked operation on raw buffer in expression}}
+
+  auto ap2 = p;
+
+  foo(ap2[0]);  // expected-warning{{unchecked operation on raw buffer in expression}}
+
+  auto ap3 = pp;
+
+  foo(pp[0][0]); // expected-warning2{{unchecked operation on raw buffer in expression}}
+
+  auto ap4 = *pp;
+
+  foo(ap4[0]);  // expected-warning{{unchecked operation on raw buffer in expression}}
+}
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -149,6 +149,31 @@
 
   const Stmt *getBaseStmt() const override { return Op; }
 };
+
+/// Array subscript expressions on raw pointers as if they're arrays. Unsafe as
+/// it doesn't have any bounds checks for the array.
+class ArraySubscriptGadget : public UnsafeGadget {
+  const ArraySubscriptExpr *ASE;
+
+public:
+  ArraySubscriptGadget(const MatchFinder::MatchResult )
+  : UnsafeGadget(Kind::ArraySubscript),
+ASE(Result.Nodes.getNodeAs("arraySubscr")) {}
+
+  static bool classof(const Gadget *G) {
+return G->getKind() == Kind::ArraySubscript;
+  }
+
+  static Matcher matcher() {
+// FIXME: What if the index is integer literal 0? Should this be
+// a safe gadget in this case?
+return stmt(
+arraySubscriptExpr(hasBase(ignoringParenImpCasts(hasPointerType(
+.bind("arraySubscr"));
+  }
+
+  const Stmt *getBaseStmt() const override { return ASE; }
+};
 } // namespace
 
 // Scan the function and return a list of gadgets found with provided kits.
Index: clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
===
--- clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
+++ clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
@@ -20,6 +20,7 @@
 
 UNSAFE_GADGET(Increment)
 UNSAFE_GADGET(Decrement)
+UNSAFE_GADGET(ArraySubscript)
 
 #undef SAFE_GADGET
 #undef UNSAFE_GADGET
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136998: Try to implement lambdas with inalloca parameters by inlining the call operator function.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Might also be worth considering if we can avoid cloning here.  It should be 
possible to emit the lambda body into a separate function with a calling 
convention of your choice, and make both the call operator and the static 
invoker call it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136998

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

[PATCH] D136998: Try to implement lambdas with inalloca parameters by inlining the call operator function.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Should we try to use this codepath for variadic lambdas as well?

Do we want to try to unify our cloning code?  
CodeGenFunction::GenerateVarArgsThunk has code doing something similar.  (It's 
at least worth comparing to see if you're doing something significantly 
different...)




Comment at: clang/lib/CodeGen/CGClass.cpp:3063
+// Don't copy the %this argument.
+if (I.getName().equals("this")) {
+  VMap[] = llvm::Constant::getNullValue(I.getType());

Checking the name doesn't work; -fdiscard-value-names is on by default in 
Release builds


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136998

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

[PATCH] D137058: [Driver] [Modules] Support -fsave-std-c++-module-file (1/2)

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

> What was the objection to "-fc++-module-filename[=]" ?

I guess it reads a bit awkwardly when you aren't providing the filename/want 
the default filename?

> GCC has "-fmodule-only"

Hmm, I don't mind that too much (& as you say, '-fobject-only' - though that 
flag is maybe too vague?) - but it does mean we'd need a separate flag to name 
the .pcm output file, because that flag ('-fmodule-only') wouldn't be present 
in all cases where the pcm is generated, only when it's pcm-but-no-object. 
Maybe less "exclusionary" flag names and more explicit (like 
'-fbuild-the-binary-module' and '-fbuild-the-object'). I guess most C++ GCC 
options ( https://gcc.gnu.org/onlinedocs/gcc/Option-Summary.html ) don't 
include "C++" in them, but the modules ones mostly include "module".

We could get out in front, planning for the world in which BMIs are really only 
the interface (& whatever else we want to carry for optimizing /use/ of that 
interface, but not complete enough to be usable for generating the modules 
object) and use `-fmodule-interface[=]` (skipping the 'binary' part) and 
`-fmodule-implementation` or `-fmodule-object` (which could go either way - 
default on or off, but provide `-fno-module-object` to do the "generate PCM 
only")?

Only other minor thing might be singular V plural - some of Clang's flags are 
`-fmodules` and others are `-fmodule` - any sense of what's likely to work 
better? (be nice to unify/standardize on one or the other, I can see having 
variation there might make for frustrating usability trying to remember which 
is which)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137058

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

[PATCH] D136998: Try to implement lambdas with inalloca parameters by inlining the call operator function.

[Amy Huang via Phabricator via cfe-commits]

akhuang added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:3590-3593
+  getTarget().getCXXABI().isMicrosoft() &&
+  llvm::any_of(cast(FD)->parameters(), [&](ParmVarDecl 
*P) {
+return isInAllocaArgument(getCXXABI(), P->getType());
+})) {

rnk wrote:
> akhuang wrote:
> > akhuang wrote:
> > > rnk wrote:
> > > > akhuang wrote:
> > > > > rnk wrote:
> > > > > > For simplicity, what if we always emitted the call operator for all 
> > > > > > lambda static invokers into the IR first? So, this logic would then 
> > > > > > become almost exactly the same as the emitCXXStructor logic above.
> > > > > > 
> > > > > > Later, in EmitLambdaStaticInvokeBody, we can detect the inalloca 
> > > > > > case and start the cloning.
> > > > > > For simplicity, what if we always emitted the call operator for all 
> > > > > > lambda static invokers into the IR first? So, this logic would then 
> > > > > > become almost exactly the same as the emitCXXStructor logic above.
> > > > > 
> > > > > Do you know where this should happen? I couldn't really figure out a 
> > > > > place other than here for emitting the call operator. 
> > > > > 
> > > > > If I do the cloning inside the normal EmitLambdaStaticInvokeBody path 
> > > > > it's a bit annoying because StartFunction/EndFunction get called 
> > > > > before/after cloning.
> > > > > Do you know where this should happen? I couldn't really figure out a 
> > > > > place other than here for emitting the call operator.
> > > > 
> > > > Yes, I think you should do that here, just like we do for constructors. 
> > > > If it's a hack, it's one we already have. The main impact is that we 
> > > > emit the call operator in the IR first. That may require updating some 
> > > > tests, but it's nice to do the same thing on all platforms, and we'd 
> > > > need to do it to handle forwarding varargs lambdas anyway, which are 
> > > > present on all platforms.
> > > > 
> > > > I also think it's OK to delete all the IR from StartFunction after its 
> > > > been generated, that doesn't seem like a big deal. How does the varargs 
> > > > cloning logic handle this situation?
> > > Oh, ok, I see what you mean. 
> > > 
> > > I'll try to upload a version with the cloning function inside 
> > > EmitLambdaStaticInvokeBody. I think there's some stuff in Start/End 
> > > Function that prevent you from simply deleting the code. (I don't think 
> > > it's an issue for the varargs cloning because that isn't called within 
> > > `EmitGlobalFunctionDefinition`. )
> > Actually, hm, trying to do the function cloning inside 
> > `EmitLambdaStaticInvokeBody`/`GenerateCode` might not work because 
> > `FinishFunction` does various things like emit the return statement, which 
> > won't work if we just cloned the function.
> If it doesn't work at all, I'm OK with doing this here, but I would like to 
> try to move as much logic as possible out of `EmitGlobalDefinition`.
Yeah, it doesn't seem ideal.. I guess the logic can also be moved into 
`EmitGlobalFunctionDefinition` (or a new CodeGenModule wrapper function) but 
that doesn't seem much better.

I don't think cloning the function in `GenerateCode` between StartFunction and 
FinishFunction will work though. Otherwise maybe can go back to trying a 
different method like changing the CXXMethodDecl from the call op and emitting 
it through the normal codegen path. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136998

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

[PATCH] D137313: [NFC] Remove redundant loads when has_device_addr is used.

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 updated this revision to Diff 473084.
jyu2 added a comment.
Herald added a subscriber: openmp-commits.

Thanks Alexey.  New runtime test is added.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137313

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/target_has_device_addr_codegen.cpp
  clang/test/OpenMP/target_has_device_addr_codegen_01.cpp
  openmp/libomptarget/test/mapping/has_device_addr.cpp

Index: openmp/libomptarget/test/mapping/has_device_addr.cpp
===
--- openmp/libomptarget/test/mapping/has_device_addr.cpp
+++ openmp/libomptarget/test/mapping/has_device_addr.cpp
@@ -25,9 +25,60 @@
   }
 };
 
+void poo() {
+  short a = 1;
+  short  = a;
+
+  #pragma omp target data map(tofrom : ar) use_device_addr(ar)
+  {
+#pragma omp target has_device_addr(ar)
+{
+  ar = 222;
+  // CHECK: 222
+  printf("%hd %p\n", ar, ); // 222 p2
+}
+  }
+  // CHECK: 222
+  printf("%hd %p\n", ar, ); // 222 p1
+}
+
+void noo() {
+  short *b = (short *)malloc(sizeof(short));
+  short * = b;
+  br = br - 1;
+
+ br[1] = 111;
+#pragma omp target data map(tofrom : br[1]) use_device_addr(br[1])
+#pragma omp target has_device_addr(br[1])
+  {
+br[1] = 222;
+// CHECK: 222
+printf("%hd %p %p %p\n", br[1], br, [1], );
+  }
+  // CHECK: 222
+  printf("%hd %p %p %p\n", br[1], br, [1], );
+}
+
+void ooo() {
+  short a = 1;
+
+#pragma omp target data map(tofrom : a) use_device_addr(a)
+#pragma omp target has_device_addr(a)
+  {
+a = 222;
+// CHECK: 222
+printf("%hd %p\n", a, );
+  }
+  // CHECK: 222
+  printf("%hd %p\n", a, );
+}
+
 int main() {
   view a;
   a.foo();
+  poo();
+  noo();
+  ooo();
   // CHECK: PASSED
   printf("PASSED\n");
 }
Index: clang/test/OpenMP/target_has_device_addr_codegen_01.cpp
===
--- clang/test/OpenMP/target_has_device_addr_codegen_01.cpp
+++ clang/test/OpenMP/target_has_device_addr_codegen_01.cpp
@@ -62,80 +62,77 @@
 // CHECK-NEXT:store ptr [[TMP3]], ptr [[TMP]], align 8
 // CHECK-NEXT:[[TMP4:%.*]] = load ptr, ptr [[PTR]], align 8
 // CHECK-NEXT:[[TMP5:%.*]] = load ptr, ptr [[TMP]], align 8
-// CHECK-NEXT:[[TMP6:%.*]] = load ptr, ptr [[PTR]], align 8
-// CHECK-NEXT:[[TMP7:%.*]] = load ptr, ptr [[TMP]], align 8
-// CHECK-NEXT:[[ARRAYDECAY:%.*]] = getelementptr inbounds [4 x float], ptr [[ARR]], i64 0, i64 0
-// CHECK-NEXT:[[TMP8:%.*]] = getelementptr inbounds [6 x ptr], ptr [[DOTOFFLOAD_BASEPTRS]], i32 0, i32 0
-// CHECK-NEXT:store ptr [[A]], ptr [[TMP8]], align 8
-// CHECK-NEXT:[[TMP10:%.*]] = getelementptr inbounds [6 x ptr], ptr [[DOTOFFLOAD_PTRS]], i32 0, i32 0
-// CHECK-NEXT:store ptr [[A]], ptr [[TMP10]], align 8
-// CHECK-NEXT:[[TMP12:%.*]] = getelementptr inbounds [6 x ptr], ptr [[DOTOFFLOAD_MAPPERS]], i64 0, i64 0
-// CHECK-NEXT:store ptr null, ptr [[TMP12]], align 8
-// CHECK-NEXT:[[TMP13:%.*]] = getelementptr inbounds [6 x ptr], ptr [[DOTOFFLOAD_BASEPTRS]], i32 0, i32 1
-// CHECK-NEXT:store ptr [[TMP6]], ptr [[TMP13]], align 8
-// CHECK-NEXT:[[TMP15:%.*]] = getelementptr inbounds [6 x ptr], ptr [[DOTOFFLOAD_PTRS]], i32 0, i32 1
-// CHECK-NEXT:store ptr [[TMP6]], ptr [[TMP15]], align 8
-// CHECK-NEXT:[[TMP17:%.*]] = getelementptr inbounds [6 x ptr], ptr [[DOTOFFLOAD_MAPPERS]], i64 0, i64 1
+// CHECK-NEXT:[[TMP6:%.*]] = getelementptr inbounds [6 x ptr], ptr [[DOTOFFLOAD_BASEPTRS]], i32 0, i32 0
+// CHECK-NEXT:store ptr [[A]], ptr [[TMP6]], align 8
+// CHECK-NEXT:[[TMP7:%.*]] = getelementptr inbounds [6 x ptr], ptr [[DOTOFFLOAD_PTRS]], i32 0, i32 0
+// CHECK-NEXT:store ptr [[A]], ptr [[TMP7]], align 8
+// CHECK-NEXT:[[TMP8:%.*]] = getelementptr inbounds [6 x ptr], ptr [[DOTOFFLOAD_MAPPERS]], i64 0, i64 0
+// CHECK-NEXT:store ptr null, ptr [[TMP8]], align 8
+// CHECK-NEXT:[[TMP9:%.*]] = getelementptr inbounds [6 x ptr], ptr [[DOTOFFLOAD_BASEPTRS]], i32 0, i32 1
+// CHECK-NEXT:store ptr [[TMP4]], ptr [[TMP9]], align 8
+// CHECK-NEXT:[[TMP10:%.*]] = getelementptr inbounds [6 x ptr], ptr [[DOTOFFLOAD_PTRS]], i32 0, i32 1
+// CHECK-NEXT:store ptr [[TMP4]], ptr [[TMP10]], align 8
+// CHECK-NEXT:[[TMP11:%.*]] = getelementptr inbounds [6 x ptr], ptr [[DOTOFFLOAD_MAPPERS]], i64 0, i64 1
+// CHECK-NEXT:store ptr null, ptr [[TMP11]], align 8
+// CHECK-NEXT:[[TMP12:%.*]] = getelementptr inbounds [6 x ptr], ptr [[DOTOFFLOAD_BASEPTRS]], i32 0, i32 2
+// CHECK-NEXT:store ptr [[TMP5]], ptr [[TMP12]], align 8
+// CHECK-NEXT:[[TMP13:%.*]] = getelementptr inbounds [6 x ptr], ptr [[DOTOFFLOAD_PTRS]], i32 0, i32 2
+// CHECK-NEXT:store ptr [[TMP5]], ptr [[TMP13]], align 8
+// CHECK-NEXT:[[TMP14:%.*]] = getelementptr inbounds [6 x ptr], ptr [[DOTOFFLOAD_MAPPERS]], i64 0, i64 2
+// CHECK-NEXT:store ptr null, ptr 

[PATCH] D137287: [Test] Fix CHECK typo.

[Zequan Wu via Phabricator via cfe-commits]

zequanwu added a comment.

In D137287#3905474 , @probinson wrote:

> Nice work!
> have you verified that all the modified tests pass? naively it looks like 
> you'd need at least 3 different targets to run them all (windows, 
> webassembly, powerpc)

Those are target independent tests. I'm able to run and pass those tests in my 
linux machine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137287

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

[PATCH] D131919: Move googletest to the third-party directory

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a comment.
Herald added a subscriber: Moerafaat.

@probinson Does this latest update look better?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131919

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

[PATCH] D90121: clang-format: Add a consumer to diagnostics engine

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

The issue that caused this issue was fix by the fix in 
https://reviews.llvm.org/rGa92cf5a5a0cd01145f8db2ae09334a8b43a1271b , from the 
clang-format perspective I don't really feel we need to revisit bringing in 
these libraries.

I think I proved that moving everything to lib/Basic isn't always the best way 
to go, if keeping the dependencies low (if speed of building the tools is of 
concern, which it is for me at least)

Ultimately if there is a better way for this to be structured, it needs to be 
handled by someone who is the owner of clang/Basic and/or llvm/Support, I don't 
feel like our team is in the best position to drive that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90121

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

[PATCH] D136998: Try to implement lambdas with inalloca parameters by inlining the call operator function.

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a subscriber: efriedma.
rnk added a comment.

+@efriedma, can you review this as a Clang CodeGen owner, according to the 
recently updated CodeOwners.rst file?
https://github.com/llvm/llvm-project/blob/main/clang/CodeOwners.rst#clang-llvm-ir-generation




Comment at: clang/lib/CodeGen/CodeGenModule.cpp:3590-3593
+  getTarget().getCXXABI().isMicrosoft() &&
+  llvm::any_of(cast(FD)->parameters(), [&](ParmVarDecl 
*P) {
+return isInAllocaArgument(getCXXABI(), P->getType());
+})) {

akhuang wrote:
> akhuang wrote:
> > rnk wrote:
> > > akhuang wrote:
> > > > rnk wrote:
> > > > > For simplicity, what if we always emitted the call operator for all 
> > > > > lambda static invokers into the IR first? So, this logic would then 
> > > > > become almost exactly the same as the emitCXXStructor logic above.
> > > > > 
> > > > > Later, in EmitLambdaStaticInvokeBody, we can detect the inalloca case 
> > > > > and start the cloning.
> > > > > For simplicity, what if we always emitted the call operator for all 
> > > > > lambda static invokers into the IR first? So, this logic would then 
> > > > > become almost exactly the same as the emitCXXStructor logic above.
> > > > 
> > > > Do you know where this should happen? I couldn't really figure out a 
> > > > place other than here for emitting the call operator. 
> > > > 
> > > > If I do the cloning inside the normal EmitLambdaStaticInvokeBody path 
> > > > it's a bit annoying because StartFunction/EndFunction get called 
> > > > before/after cloning.
> > > > Do you know where this should happen? I couldn't really figure out a 
> > > > place other than here for emitting the call operator.
> > > 
> > > Yes, I think you should do that here, just like we do for constructors. 
> > > If it's a hack, it's one we already have. The main impact is that we emit 
> > > the call operator in the IR first. That may require updating some tests, 
> > > but it's nice to do the same thing on all platforms, and we'd need to do 
> > > it to handle forwarding varargs lambdas anyway, which are present on all 
> > > platforms.
> > > 
> > > I also think it's OK to delete all the IR from StartFunction after its 
> > > been generated, that doesn't seem like a big deal. How does the varargs 
> > > cloning logic handle this situation?
> > Oh, ok, I see what you mean. 
> > 
> > I'll try to upload a version with the cloning function inside 
> > EmitLambdaStaticInvokeBody. I think there's some stuff in Start/End 
> > Function that prevent you from simply deleting the code. (I don't think 
> > it's an issue for the varargs cloning because that isn't called within 
> > `EmitGlobalFunctionDefinition`. )
> Actually, hm, trying to do the function cloning inside 
> `EmitLambdaStaticInvokeBody`/`GenerateCode` might not work because 
> `FinishFunction` does various things like emit the return statement, which 
> won't work if we just cloned the function.
If it doesn't work at all, I'm OK with doing this here, but I would like to try 
to move as much logic as possible out of `EmitGlobalDefinition`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136998

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

[PATCH] D136998: Try to implement lambdas with inalloca parameters by inlining the call operator function.

[Amy Huang via Phabricator via cfe-commits]

akhuang updated this revision to Diff 473082.
akhuang added a comment.

moved some stuff around


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136998

Files:
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGenCXX/inalloca-lambda.cpp

Index: clang/test/CodeGenCXX/inalloca-lambda.cpp
===
--- clang/test/CodeGenCXX/inalloca-lambda.cpp
+++ clang/test/CodeGenCXX/inalloca-lambda.cpp
@@ -1,7 +1,4 @@
-// RUN: not %clang_cc1 -triple i686-windows-msvc -emit-llvm -o /dev/null %s  2>&1 | FileCheck %s
-
-// PR28299
-// CHECK: error: cannot compile this forwarded non-trivially copyable parameter yet
+// RUN: %clang_cc1 -triple i686-windows-msvc -emit-llvm -o - %s  2>&1 | FileCheck %s
 
 class A {
   A(const A &);
@@ -9,3 +6,7 @@
 typedef void (*fptr_t)(A);
 fptr_t fn1() { return [](A) {}; }
 
+// CHECK: define internal x86_thiscallcc void @"?__invoke@@?0??fn1@@YAP6AXVA@@@ZXZ@CA?A?@@0@Z"
+// CHECK-SAME: (ptr inalloca(<{ %class.A, [3 x i8] }>) %0)
+// CHECK: %1 = getelementptr inbounds <{ %class.A, [3 x i8] }>, ptr %0, i32 0, i32 0
+// CHECK: ret void
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -3554,6 +3554,18 @@
 EmitGlobalFunctionDefinition(GD, GV);
 }
 
+static bool isInAllocaArg(CGCXXABI , QualType T) {
+  const CXXRecordDecl *RD = T->getAsCXXRecordDecl();
+  return RD && ABI.getRecordArgABI(RD) == CGCXXABI::RAA_DirectInMemory;
+}
+
+static bool hasInAllocaArg(const TargetInfo , CGCXXABI , const CXXMethodDecl *MD) {
+  return TI.getCXXABI().isMicrosoft() && 
+llvm::any_of(MD->parameters(), [&](ParmVarDecl *P) {
+return isInAllocaArg(CGABI, P->getType());
+  });
+}
+
 void CodeGenModule::EmitGlobalDefinition(GlobalDecl GD, llvm::GlobalValue *GV) {
   const auto *D = cast(GD.getDecl());
 
@@ -3580,6 +3592,14 @@
   // This is necessary for the generation of certain thunks.
   if (isa(Method) || isa(Method))
 ABI->emitCXXStructor(GD);
+  // Special path for emitting lambda static invokers with inalloca parameters.
+  else if (Method->isLambdaStaticInvoker() && 
+   hasInAllocaArg(getTarget(), getCXXABI(), Method)) {
+// Emit the call operator definition before emitting a static invoker.
+const CXXMethodDecl *CallOp = Method->getParent()->getLambdaCallOperator();
+EmitGlobalFunctionDefinition(GlobalDecl(CallOp), nullptr);
+CodeGenFunction(*this).EmitClonedLambdaStaticInvoke(Method);
+  }
   else if (FD->isMultiVersion())
 EmitMultiVersionFunctionDefinition(GD, GV);
   else
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -2223,6 +2223,8 @@
   void EmitLambdaBlockInvokeBody();
   void EmitLambdaDelegatingInvokeBody(const CXXMethodDecl *MD);
   void EmitLambdaStaticInvokeBody(const CXXMethodDecl *MD);
+  void EmitClonedLambdaStaticInvoke(const CXXMethodDecl *MD);
+
   void EmitLambdaVLACapture(const VariableArrayType *VAT, LValue LV) {
 EmitStoreThroughLValue(RValue::get(VLASizeMap[VAT->getSizeExpr()]), LV);
   }
Index: clang/lib/CodeGen/CGClass.cpp
===
--- clang/lib/CodeGen/CGClass.cpp
+++ clang/lib/CodeGen/CGClass.cpp
@@ -26,8 +26,10 @@
 #include "clang/Basic/CodeGenOptions.h"
 #include "clang/Basic/TargetBuiltins.h"
 #include "clang/CodeGen/CGFunctionInfo.h"
+#include "llvm/IR/Instructions.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/Metadata.h"
+#include "llvm/Transforms/Utils/Cloning.h"
 #include "llvm/Transforms/Utils/SanitizerStats.h"
 
 using namespace clang;
@@ -3044,3 +3046,38 @@
 
   EmitLambdaDelegatingInvokeBody(MD);
 }
+
+void CodeGenFunction::EmitClonedLambdaStaticInvoke(const CXXMethodDecl *MD) {
+  llvm::Function *Fn =
+cast(CGM.GetAddrOfFunction(GlobalDecl(MD)));
+
+  const CXXMethodDecl *CallOp = MD->getParent()->getLambdaCallOperator();
+  llvm::Function *CallOpFn =
+cast(CGM.GetAddrOfFunction(GlobalDecl(CallOp)));
+
+  // Clone from call operator, which we've made sure is already emitted.
+  llvm::ValueToValueMapTy VMap;
+
+  for (llvm::Argument  : CallOpFn->args()) {
+// Don't copy the %this argument.
+if (I.getName().equals("this")) {
+  VMap[] = llvm::Constant::getNullValue(I.getType());
+} else {
+  // Try to map the inalloca arg from the call op fn to the invoker fn.
+  for (llvm::Argument  : Fn->args()) {
+if (DestI.getType() == I.getType()) {
+  VMap[] = 
+  break;
+}
+  }
+}
+  }
+
+  Fn->setAttributes(CallOpFn->getAttributes());

[PATCH] D137058: [Driver] [Modules] Support -fsave-std-c++-module-file (1/2)

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

In D137058#3906579 , @dblaikie wrote:

> I realize I got this jumbled up and the thread about "why do we need to name 
> things" is meant to be over in D137059  
> (sorry @ben.boeckel :/ I know this is all confusing to follow at the best of 
> times) so I'll pick that up there.
>
> But maybe a relevant question /here/ (maybe @iains has some context here, and 
> @ben.boeckel ): What are GCC's (& any other implementations) command line 
> interfaces for things like this? How're the command line flags spelled? To 
> see about inspiration for this.

.. the state I have is ...
As you note there are two parts to this:

1. the ability to emit a BMI and object from one invocation of the compiler 
(which can be done as in this series by the driver - or with alternate patches 
in the FE - which allows for the improvements we want in the BMI) 2, How to 
name the BMI.
- in my experimentation with GCC, for this part, it defers the decision 
until the module name is known, and then queries an interface to ask for the 
BMI filename for that named module.  The interface can be internal (e.g. 
choosing a simple default like source.pcm) or it could get the mapping from 
some other place, like a build system.  The FE side of the transaction does not 
need to know how the name was chosen (so it _could_ be specified from a 
command-line flag).
  - but, AFAIR, GCC does not currently have such a flag.

> (judging from the discourse thread with @rsmith it seems like "the ability to 
> generate a .o from a .pcm" is disfavored, he didn't outright say "we should 
> remove that functionality" but pretty close to it - in favor of ".cppm -> 
> {.o, .pcm}" and ".ccpm -> .o" + ".cppm -> .pcm" (without the ability to 
> generate the .o from the .pcm) - in which case we could change the existing 
> driver behavior sooner or later to address that third action (.cppm -> .pcm 
> but it's only a minimal pcm).

This is the strategy my draft patches for the FE aims for (apologies, for 
repeatedly mentioning a patch set that is not yet up for review ...)

Maybe we should have two flags one that says "produce a .pcm" (which we already 
have `--precompile` for that) and another that says "produce a .o" (I guess 
that's the default, so maybe we want a way to opt /out/ of that behavior?))

> Eh, sorry, just talking myself around in circles.
> Currently we have:
> `.cppm` -> `.o` (no extra flags)
> `.cppm` -> `.pcm` (`--precompile`)
> `.cppm` -> `.pcm` + `.o` (unsupported)
>
> I'm not sure that `--precompile` is the best flag name to be inspired by 
> (it's unqualified by any `-f` or other prefix, which usually feels a bit 
> weird, and it's a very generic term, doesn't mention modules, etc) -

GCC has "-fmodule-only" for which I have a patch (also unpublished)  that 
aliases that to --precompile in the driver.
(I think we could possibly see a reason to have -fobject-only for symmetry and 
to cover the cases you mention here)

> so I can appreciate the `-fsave-std-c++-module-file` name here, but it does 
> sound a bit verbose? Wouldn't mind hearing other people's thoughts on flag 
> names, especially any prior art/other implementation choices we could look 
> for for inspiration.

What was the objection to "-fc++-module-filename[=]" ?

As noted above, if my memory is not too faulty, GCC does not have this specific 
[name the BMI] flag yet (I think this is the case because GCC essentially uses 
a simple in-process P1184  interface as the 
fallback when there's no external build system), then this flag name is up for 
grabs - and what is chosen here could, presumably, be implemented for GCC too.

Actually, in some ways clang does something pretty similar in that there's the 
in-process module-mapper - which is where the filenames probably should be 
decided, we just tend to bypass it with specific command line input for output 
filenames (which does not work where there are two names needed, except by 
providing some simplified fallback - like the "just change the extension" 
approach).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137058

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

[PATCH] D137334: [clang][dataflow] Generalize custom comparison to return tri-value result.

[Yitzhak Mandelbaum 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 rGc0725865b188: [clang][dataflow] Generalize custom comparison 
to return tri-value result. (authored by ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137334

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  
clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
  clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -351,24 +351,27 @@
 }
   }
 
-  bool compareEquivalent(QualType Type, const Value ,
- const Environment , const Value ,
- const Environment ) override {
+  ComparisonResult compare(QualType Type, const Value ,
+   const Environment , const Value ,
+   const Environment ) override {
 const auto *Decl = Type->getAsCXXRecordDecl();
 if (Decl == nullptr || Decl->getIdentifier() == nullptr ||
 Decl->getName() != "SpecialBool")
-  return false;
+  return ComparisonResult::Unknown;
 
 auto *IsSet1 = cast_or_null(Val1.getProperty("is_set"));
+auto *IsSet2 = cast_or_null(Val2.getProperty("is_set"));
 if (IsSet1 == nullptr)
-  return true;
+  return IsSet2 == nullptr ? ComparisonResult::Same
+   : ComparisonResult::Different;
 
-auto *IsSet2 = cast_or_null(Val2.getProperty("is_set"));
 if (IsSet2 == nullptr)
-  return false;
+  return ComparisonResult::Different;
 
 return Env1.flowConditionImplies(*IsSet1) ==
-   Env2.flowConditionImplies(*IsSet2);
+   Env2.flowConditionImplies(*IsSet2)
+   ? ComparisonResult::Same
+   : ComparisonResult::Different;
   }
 
   // Always returns `true` to accept the `MergedVal`.
@@ -509,18 +512,19 @@
 }
   }
 
-  bool compareEquivalent(QualType Type, const Value ,
- const Environment , const Value ,
- const Environment ) override {
+  ComparisonResult compare(QualType Type, const Value ,
+   const Environment , const Value ,
+   const Environment ) override {
 // Nothing to say about a value that does not model an `OptionalInt`.
 if (!Type->isRecordType() ||
 Type->getAsCXXRecordDecl()->getQualifiedNameAsString() != "OptionalInt")
-  return false;
+  return ComparisonResult::Unknown;
 
 auto *Prop1 = Val1.getProperty("has_value");
 auto *Prop2 = Val2.getProperty("has_value");
 assert(Prop1 != nullptr && Prop2 != nullptr);
-return areEquivalentValues(*Prop1, *Prop2);
+return areEquivalentValues(*Prop1, *Prop2) ? ComparisonResult::Same
+   : ComparisonResult::Different;
   }
 
   bool merge(QualType Type, const Value , const Environment ,
@@ -1182,12 +1186,12 @@
 }
   }
 
-  bool compareEquivalent(QualType Type, const Value ,
- const Environment , const Value ,
- const Environment ) override {
+  ComparisonResult compare(QualType Type, const Value ,
+   const Environment , const Value ,
+   const Environment ) override {
 // Changes to a sound approximation, which allows us to test whether we can
 // (soundly) converge for some loops.
-return false;
+return ComparisonResult::Unknown;
   }
 };
 
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -208,7 +208,7 @@
 }
 
 /// Returns true if and only if `Type` is an optional type.
-bool IsOptionalType(QualType Type) {
+bool isOptionalType(QualType Type) {
   if (!Type->isRecordType())
 return false;
   // FIXME: Optimize this by avoiding the `getQualifiedNameAsString` call.
@@ -222,7 +222,7 @@
 /// For example, if `Type` is `optional>`, the result of this
 /// function will be 2.
 int countOptionalWrappers(const ASTContext , QualType Type) {
-  if (!IsOptionalType(Type))
+  if (!isOptionalType(Type))
 return 0;
   return 1 + countOptionalWrappers(
  ASTCtx,
@@ -720,12 

[clang] c072586 - [clang][dataflow] Generalize custom comparison to return tri-value result.

[Yitzhak Mandelbaum via cfe-commits]

Author: Yitzhak Mandelbaum
Date: 2022-11-03T23:31:20Z
New Revision: c0725865b188f71f904ecd4dac56ef37268b30d2

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

LOG: [clang][dataflow] Generalize custom comparison to return tri-value result.

Currently, the API for a model's custom value comparison returns a
boolean. Therefore, models cannot distinguish between situations where the
values are recognized by the model and different and those where the values are
just not recognized.  This patch changes the return value to a tri-valued enum,
allowing models to express "don't know".

This patch is essentially a NFC -- no practical differences result from this
change in this patch. But, it prepares for future patches (particularly,
upcoming patches for widening) which will take advantage of the new flexibility.

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

Added: 


Modified: 
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h

clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

Removed: 




diff  --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h 
b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
index efea46b4a0c5b..e362d79263ff2 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -48,6 +48,13 @@ enum class SkipPast {
   ReferenceThenPointer,
 };
 
+/// Indicates the result of a tentative comparison.
+enum class ComparisonResult {
+  Same,
+  Different,
+  Unknown,
+};
+
 /// Holds the state of the program (store and heap) at a given program point.
 ///
 /// WARNING: Symbolic values that are created by the environment for static
@@ -62,7 +69,11 @@ class Environment {
   public:
 virtual ~ValueModel() = default;
 
-/// Returns true if and only if `Val1` is equivalent to `Val2`.
+/// Returns:
+///   `Same`: `Val1` is equivalent to `Val2`, according to the model.
+///   `Different`: `Val1` is distinct from `Val2`, according to the model.
+///   `Unknown`: The model can't determine a relationship between `Val1` 
and
+///`Val2`.
 ///
 /// Requirements:
 ///
@@ -72,16 +83,16 @@ class Environment {
 ///
 ///  `Val1` and `Val2` must be assigned to the same storage location in
 ///  `Env1` and `Env2` respectively.
-virtual bool compareEquivalent(QualType Type, const Value ,
-   const Environment , const Value ,
-   const Environment ) {
+virtual ComparisonResult compare(QualType Type, const Value ,
+ const Environment , const Value 
,
+ const Environment ) {
   // FIXME: Consider adding QualType to StructValue and removing the Type
   // argument here.
   //
-  // FIXME: default to a sound comparison and/or expand the comparison 
logic
-  // built into the framework to support broader forms of equivalence than
-  // strict pointer equality.
-  return true;
+  // FIXME: default to a sound comparison (`Unknown`) and/or expand the
+  // comparison logic built into the framework to support broader forms of
+  // equivalence than strict pointer equality.
+  return ComparisonResult::Same;
 }
 
 /// Modifies `MergedVal` to approximate both `Val1` and `Val2`. This could

diff  --git 
a/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
 
b/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
index 66aabb531a213..b053a10327c3f 100644
--- 
a/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
+++ 
b/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
@@ -54,9 +54,9 @@ class UncheckedOptionalAccessModel
 
   void transfer(const CFGElement *Elt, NoopLattice , Environment );
 
-  bool compareEquivalent(QualType Type, const Value ,
- const Environment , const Value ,
- const Environment ) override;
+  ComparisonResult compare(QualType Type, const Value ,
+   const Environment , const Value ,
+   const Environment ) override;
 
   bool merge(QualType Type, const Value , const Environment ,
  const Value , const Environment , Value ,

diff  --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp 

[PATCH] D136998: Try to implement lambdas with inalloca parameters by inlining the call operator function.

[Amy Huang via Phabricator via cfe-commits]

akhuang added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:3590-3593
+  getTarget().getCXXABI().isMicrosoft() &&
+  llvm::any_of(cast(FD)->parameters(), [&](ParmVarDecl 
*P) {
+return isInAllocaArgument(getCXXABI(), P->getType());
+})) {

akhuang wrote:
> rnk wrote:
> > akhuang wrote:
> > > rnk wrote:
> > > > For simplicity, what if we always emitted the call operator for all 
> > > > lambda static invokers into the IR first? So, this logic would then 
> > > > become almost exactly the same as the emitCXXStructor logic above.
> > > > 
> > > > Later, in EmitLambdaStaticInvokeBody, we can detect the inalloca case 
> > > > and start the cloning.
> > > > For simplicity, what if we always emitted the call operator for all 
> > > > lambda static invokers into the IR first? So, this logic would then 
> > > > become almost exactly the same as the emitCXXStructor logic above.
> > > 
> > > Do you know where this should happen? I couldn't really figure out a 
> > > place other than here for emitting the call operator. 
> > > 
> > > If I do the cloning inside the normal EmitLambdaStaticInvokeBody path 
> > > it's a bit annoying because StartFunction/EndFunction get called 
> > > before/after cloning.
> > > Do you know where this should happen? I couldn't really figure out a 
> > > place other than here for emitting the call operator.
> > 
> > Yes, I think you should do that here, just like we do for constructors. If 
> > it's a hack, it's one we already have. The main impact is that we emit the 
> > call operator in the IR first. That may require updating some tests, but 
> > it's nice to do the same thing on all platforms, and we'd need to do it to 
> > handle forwarding varargs lambdas anyway, which are present on all 
> > platforms.
> > 
> > I also think it's OK to delete all the IR from StartFunction after its been 
> > generated, that doesn't seem like a big deal. How does the varargs cloning 
> > logic handle this situation?
> Oh, ok, I see what you mean. 
> 
> I'll try to upload a version with the cloning function inside 
> EmitLambdaStaticInvokeBody. I think there's some stuff in Start/End Function 
> that prevent you from simply deleting the code. (I don't think it's an issue 
> for the varargs cloning because that isn't called within 
> `EmitGlobalFunctionDefinition`. )
Actually, hm, trying to do the function cloning inside 
`EmitLambdaStaticInvokeBody`/`GenerateCode` might not work because 
`FinishFunction` does various things like emit the return statement, which 
won't work if we just cloned the function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136998

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

[PATCH] D137375: [AIX][pg] Add Correct Search Paths for Profiled Libraries

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

This needs a clang/test/Driver test. And, does this work with --sysroot= ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137375

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

[PATCH] D137107: Allow MS extension: support of constexpr with __declspec(dllimport).

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In D137107#3905443 , @zahiraam wrote:

>   extern int __declspec(dllimport) next(int n);
>   int main () {
> extern int _declspec(dllimport) val;
> constexpr int& val_ref = val;
> int i = next(val_ref);
> return i;
>   } 
>
> @rnk Shouldn't this run?

Yes, I agree, this is a bug. Clang should compile this and reference 
`__imp_next` here. However, Clang should continue producing errors when a 
dllimport symbol is used to initialize a constexpr global variable, which was 
one of the other cases mentioned in the initial report.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137107

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

[PATCH] D137375: [AIX][pg] Add Correct Search Paths for Profiled Libraries

[Michael Francis via Phabricator via cfe-commits]

francii created this revision.
Herald added a project: All.
francii requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

On AIX, profiled system libraries are stored at `/lib/profiled` and 
`/usr/lib/profiled`. When compiling with `-pg`, we want to link against 
libraries in those directories. This PR modifies the AIX toolchain to add those 
directories to the linker search paths.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137375

Files:
  clang/lib/Driver/ToolChains/AIX.cpp


Index: clang/lib/Driver/ToolChains/AIX.cpp
===
--- clang/lib/Driver/ToolChains/AIX.cpp
+++ clang/lib/Driver/ToolChains/AIX.cpp
@@ -250,6 +250,11 @@
   CmdArgs.push_back("-lm");
 
 CmdArgs.push_back("-lc");
+
+if (Args.hasArg(options::OPT_pg)) {
+  CmdArgs.push_back("-L/lib/profiled");
+  CmdArgs.push_back("-L/usr/lib/profiled");
+}
   }
 
   const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());


Index: clang/lib/Driver/ToolChains/AIX.cpp
===
--- clang/lib/Driver/ToolChains/AIX.cpp
+++ clang/lib/Driver/ToolChains/AIX.cpp
@@ -250,6 +250,11 @@
   CmdArgs.push_back("-lm");
 
 CmdArgs.push_back("-lc");
+
+if (Args.hasArg(options::OPT_pg)) {
+  CmdArgs.push_back("-L/lib/profiled");
+  CmdArgs.push_back("-L/usr/lib/profiled");
+}
   }
 
   const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137268: [clang][Headers] Do not define varargs macros for __need___va_list

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/lib/Headers/stdarg.h:21
+
+#ifdef __STDARG_H
 

Maybe the following is a little more readable?

```
#ifndef __STDARG_H

#ifndef __GNUC_VA_LIST
#define __GNUC_VA_LIST 1
typedef __builtin_va_list __gnuc_va_list;
#endif

#ifdef __need___va_list
#undef __need___va_list
#else
#define __STDARG_H
[...]
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137268

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

[PATCH] D137373: [AIX][p] Add 64-bit Test Case

[Michael Francis via Phabricator via cfe-commits]

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

Add a 64-bit version of the already-existing
`-p` test case for compiler-emitted linker flags.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137373

Files:
  clang/test/Driver/aix-ld.c


Index: clang/test/Driver/aix-ld.c
===
--- clang/test/Driver/aix-ld.c
+++ clang/test/Driver/aix-ld.c
@@ -136,6 +136,33 @@
 // CHECK-LD32-PROF-NOT: "-lm"
 // CHECK-LD32-PROF: "-lc"
 
+// Check powerpc64-ibm-aix7.1.0.0, 64-bit. Enable profiling.
+// RUN: %clang %s -### 2>&1 \
+// RUN:-resource-dir=%S/Inputs/resource_dir \
+// RUN:-p \
+// RUN:--target=powerpc64-ibm-aix7.1.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:--unwindlib=libunwind \
+// RUN:   | FileCheck --check-prefix=CHECK-LD64-PROF %s
+// CHECK-LD64-PROF-NOT: warning:
+// CHECK-LD64-PROF: "-cc1" "-triple" "powerpc64-ibm-aix7.1.0.0"
+// CHECK-LD64-PROF: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-LD64-PROF: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-LD64-PROF: "{{.*}}ld{{(.exe)?}}"
+// CHECK-LD64-PROF-NOT: "-bnso"
+// CHECK-LD64-PROF: "-b64"
+// CHECK-LD64-PROF: "-bpT:0x1" "-bpD:0x11000"
+// CHECK-LD64-PROF: "[[SYSROOT]]/usr/lib{{/|}}mcrt0_64.o"
+// CHECK-LD64-PROF: "[[SYSROOT]]/usr/lib{{/|}}crti_64.o"
+// CHECK-LD64-PROF-NOT: "-lc++"
+// CHECK-LD64-PROF-NOT: "-lc++abi"
+// CHECK-LD64-PROF: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc64.a"
+// CHECK-LD64-PROF-NOT: "--as-needed"
+// CHECK-LD64-PROF: "-lunwind"
+// CHECK-LD64-PROF-NOT: "--no-as-needed"
+// CHECK-LD64-PROF-NOT: "-lm"
+// CHECK-LD64-PROF: "-lc"
+
 // Check powerpc64-ibm-aix7.1.0.0, 64-bit. Enable g-profiling.
 // RUN: %clang %s -### 2>&1 \
 // RUN:-resource-dir=%S/Inputs/resource_dir \


Index: clang/test/Driver/aix-ld.c
===
--- clang/test/Driver/aix-ld.c
+++ clang/test/Driver/aix-ld.c
@@ -136,6 +136,33 @@
 // CHECK-LD32-PROF-NOT: "-lm"
 // CHECK-LD32-PROF: "-lc"
 
+// Check powerpc64-ibm-aix7.1.0.0, 64-bit. Enable profiling.
+// RUN: %clang %s -### 2>&1 \
+// RUN:-resource-dir=%S/Inputs/resource_dir \
+// RUN:-p \
+// RUN:--target=powerpc64-ibm-aix7.1.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:--unwindlib=libunwind \
+// RUN:   | FileCheck --check-prefix=CHECK-LD64-PROF %s
+// CHECK-LD64-PROF-NOT: warning:
+// CHECK-LD64-PROF: "-cc1" "-triple" "powerpc64-ibm-aix7.1.0.0"
+// CHECK-LD64-PROF: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-LD64-PROF: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-LD64-PROF: "{{.*}}ld{{(.exe)?}}"
+// CHECK-LD64-PROF-NOT: "-bnso"
+// CHECK-LD64-PROF: "-b64"
+// CHECK-LD64-PROF: "-bpT:0x1" "-bpD:0x11000"
+// CHECK-LD64-PROF: "[[SYSROOT]]/usr/lib{{/|}}mcrt0_64.o"
+// CHECK-LD64-PROF: "[[SYSROOT]]/usr/lib{{/|}}crti_64.o"
+// CHECK-LD64-PROF-NOT: "-lc++"
+// CHECK-LD64-PROF-NOT: "-lc++abi"
+// CHECK-LD64-PROF: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc64.a"
+// CHECK-LD64-PROF-NOT: "--as-needed"
+// CHECK-LD64-PROF: "-lunwind"
+// CHECK-LD64-PROF-NOT: "--no-as-needed"
+// CHECK-LD64-PROF-NOT: "-lm"
+// CHECK-LD64-PROF: "-lc"
+
 // Check powerpc64-ibm-aix7.1.0.0, 64-bit. Enable g-profiling.
 // RUN: %clang %s -### 2>&1 \
 // RUN:-resource-dir=%S/Inputs/resource_dir \
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137372: [AIX][pg] Add 32-bit test case

[Michael Francis via Phabricator via cfe-commits]

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

Add a 32-bit version of the already-existing
pg test case for compiler-emitted linker flags.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137372

Files:
  clang/test/Driver/aix-ld.c


Index: clang/test/Driver/aix-ld.c
===
--- clang/test/Driver/aix-ld.c
+++ clang/test/Driver/aix-ld.c
@@ -136,6 +136,33 @@
 // CHECK-LD32-PROF-NOT: "-lm"
 // CHECK-LD32-PROF: "-lc"
 
+// Check powerpc-ibm-aix7.1.0.0, 32-bit. Enable g-profiling.
+// RUN: %clang %s -### 2>&1 \
+// RUN:-resource-dir=%S/Inputs/resource_dir \
+// RUN:-pg \
+// RUN:--target=powerpc-ibm-aix7.1.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:--unwindlib=libunwind \
+// RUN:   | FileCheck --check-prefix=CHECK-LD32-GPROF %s
+// CHECK-LD32-GPROF-NOT: warning:
+// CHECK-LD32-GPROF: "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"
+// CHECK-LD32-GPROF: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-LD32-GPROF: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-LD32-GPROF: "{{.*}}ld{{(.exe)?}}"
+// CHECK-LD32-GPROF-NOT: "-bnso"
+// CHECK-LD32-GPROF: "-b32"
+// CHECK-LD32-GPROF: "-bpT:0x1000" "-bpD:0x2000"
+// CHECK-LD32-GPROF: "[[SYSROOT]]/usr/lib{{/|}}gcrt0.o"
+// CHECK-LD32-GPROF: "[[SYSROOT]]/usr/lib{{/|}}crti.o"
+// CHECK-LD32-GPROF-NOT: "-lc++"
+// CHECK-LD32-GPROF-NOT: "-lc++abi"
+// CHECK-LD32-GPROF: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-GPROF-NOT: "--as-needed"
+// CHECK-LD32-GPROF: "-lunwind"
+// CHECK-LD32-GPROF-NOT: "--no-as-needed"
+// CHECK-LD32-GPROF-NOT: "-lm"
+// CHECK-LD32-GPROF: "-lc"
+
 // Check powerpc64-ibm-aix7.1.0.0, 64-bit. Enable g-profiling.
 // RUN: %clang %s -### 2>&1 \
 // RUN:-resource-dir=%S/Inputs/resource_dir \


Index: clang/test/Driver/aix-ld.c
===
--- clang/test/Driver/aix-ld.c
+++ clang/test/Driver/aix-ld.c
@@ -136,6 +136,33 @@
 // CHECK-LD32-PROF-NOT: "-lm"
 // CHECK-LD32-PROF: "-lc"
 
+// Check powerpc-ibm-aix7.1.0.0, 32-bit. Enable g-profiling.
+// RUN: %clang %s -### 2>&1 \
+// RUN:-resource-dir=%S/Inputs/resource_dir \
+// RUN:-pg \
+// RUN:--target=powerpc-ibm-aix7.1.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:--unwindlib=libunwind \
+// RUN:   | FileCheck --check-prefix=CHECK-LD32-GPROF %s
+// CHECK-LD32-GPROF-NOT: warning:
+// CHECK-LD32-GPROF: "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"
+// CHECK-LD32-GPROF: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-LD32-GPROF: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-LD32-GPROF: "{{.*}}ld{{(.exe)?}}"
+// CHECK-LD32-GPROF-NOT: "-bnso"
+// CHECK-LD32-GPROF: "-b32"
+// CHECK-LD32-GPROF: "-bpT:0x1000" "-bpD:0x2000"
+// CHECK-LD32-GPROF: "[[SYSROOT]]/usr/lib{{/|}}gcrt0.o"
+// CHECK-LD32-GPROF: "[[SYSROOT]]/usr/lib{{/|}}crti.o"
+// CHECK-LD32-GPROF-NOT: "-lc++"
+// CHECK-LD32-GPROF-NOT: "-lc++abi"
+// CHECK-LD32-GPROF: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-GPROF-NOT: "--as-needed"
+// CHECK-LD32-GPROF: "-lunwind"
+// CHECK-LD32-GPROF-NOT: "--no-as-needed"
+// CHECK-LD32-GPROF-NOT: "-lm"
+// CHECK-LD32-GPROF: "-lc"
+
 // Check powerpc64-ibm-aix7.1.0.0, 64-bit. Enable g-profiling.
 // RUN: %clang %s -### 2>&1 \
 // RUN:-resource-dir=%S/Inputs/resource_dir \
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137059: [Driver] [Modules] Introduce -fsave-std-c++-module-file= to specify the path of the module file (2/2)

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a subscriber: rsmith.
dblaikie added a comment.

In D137059#3899339 , @ben.boeckel 
wrote:

> There is another motivating factor for 1-phase: the build graph is far 
> simpler. With 2-phase, CMake will have to write out rules to perform:
>
> - source -> .bmi
> - .bmi -> .withbmi.o
> - source -> .o
>
> because we do not know if a BMI is needed or not. If it isn't we use the 
> latter. If it is, we use the former. Note that this also means we need 2 
> different `.o` filenames (as neither `make` nor `ninja` doesn't support 
> multiple rules making the same output). This also means that the collator 
> needs to generate a response file for the linker to direct which `.o` file to 
> use for each TU based on the contents.

I /think/ from @rsmith's comments in the discourse thread, we're more likely to 
skip/remove the ability to go from ".bmi" -> ".o" and possibly have 2 path 
options (this is all from @rsmith's comments on discourse) either ".cppm -> 
{.pcm, .o}" or ".cppm -> .o" + ".cppm -> .pcm" - this'd avoid the need to 
maintain full V slim pcm, there would never be a pcm that could produce a .o, 
.pcm would only be sufficient for users, not implementation.

But yeah, maybe we end up with all 3 options in the interim. Though I'd really 
like to keep the surface area as small as possible, while still allowing room 
for experimentation. Perhaps experimentation via -Xclang flags until data shows 
the options are worthwhile beyond those experiments.

Pulling in your (Ben) comment from D137058 :

> Plus the other compilers offer controls over it; why does Clang have to be 
> different?

Which compilers/flags are you referring to? Arguments from compatibility with 
GCC are relatively easy to make (though I still have more hesitance for these 
flags since there's not wide-scale adoption, and I think there's still room to 
shape the world we want to see and limit the width of the interface/variations 
we end up having to support long term) & might side-step some of the 
discussions here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137059

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

[PATCH] D136811: -Wunsafe-buffer-usage: WIP: RFC: NFC: User documentation.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/docs/SafeBuffers.rst:31
+convert large amounts of old code to conform to the warning;
+  - Attribute ``[[unsafe_buffer_usage]]`` lets you annotate custom functions as
+unsafe, while providing a safe alternative that can often be suggested by

aaron.ballman wrote:
> xazax.hun wrote:
> > aaron.ballman wrote:
> > > H that attribute name looks problematic (more below in that section).
> > With C++ modules gaining some momentum, I wonder if it would be 
> > possible/beneficial to be able to annotate module import/export 
> > declarations with this attribute to signal that the APIs and the content in 
> > a module was not yet hardened. 
> I think it makes sense on module export (the module author knows whether they 
> are buffer-safe or not), but what do you have in mind for import where the 
> user may not be in a position to make that assertion validly?
> I wonder if it would be possible/beneficial to be able to annotate module 
> import/export declarations with this attribute to signal that the APIs and 
> the content in a module was not yet hardened.

It's usually not that valuable for us to know that outside the module. If the 
module wasn't hardened, there's nothing the "caller module" can do about it, 
the recommended approach is to harden the module first, and then use the 
attribute to indicate which functions were found to be unsafe in that process 
(especially if safe alternatives were implemented).

So it's only valuable when the module *will never be hardened*, and the only 
thing the caller module can do in such case is isolate calls to that module, 
maybe cover it with a safe abstraction. In this case, yeah, I think it makes 
sense. Maybe we should consider this eventually.



Comment at: clang/docs/SafeBuffers.rst:36-37
+hardened mode where C++ classes such as ``std::vector`` and ``std::span``,
+together with their respective ``iterator`` classes, are protected
+at runtime against buffer overflows. This changes buffer overflows from
+extremely dangerous undefined behavior to predictable runtime crash.

xazax.hun wrote:
> Only buffer overflows, so comparing cases like:
> ```
> auto it1 = v1.begin();
> auto it2 = v2.begin();
> 
> if (it1 == it2) { ... }
> ```
> 
> are out of scope, right? 
> 
> The main reason I'm asking, because I am not sure how safe iterators are 
> implemented and whether the same implementation would give opportunities for 
> other safety checks without significant additional cost. 
I suspect libc++ will have such checks (or even already does, @ldionne pls 
correct me if I'm wrong).

They're still out of scope for our proposal though. But they're definitely 
useful and may deserve mentioning.



Comment at: clang/docs/SafeBuffers.rst:40-41
+  - Finally, in order to avoid bugs in newly converted code, the
+Clang static analyzer provides a checker to find misconstructed
+``std::span`` objects.
+

xazax.hun wrote:
> Isn't this overly specific? What about `string_view`s? 
Yeah I think we can cover string views as well!



Comment at: clang/docs/SafeBuffers.rst:115
+   (TODO: Will automatic fixits be able to suggest custom containers or views?)
+   (TODO: Explain how to implement such checks in a custom container?)
+

aaron.ballman wrote:
> xazax.hun wrote:
> > jkorous wrote:
> > > aaron.ballman wrote:
> > > > I think this would be a good idea, especially if you can use an example 
> > > > of a more involved (but still common) container. Like, how does someone 
> > > > harden a ring buffer?
> > > I believe that we should stick to hardening low-level primitives - all 
> > > that a ring buffer implementation has to do is to store data in 
> > > `std::array` from hardened libc++ and it's all set.
> > I think it would be great to link to some other resources like how to 
> > annotate your container such that address sanitizer can help you catch OOB 
> > accesses.
> > I believe that we should stick to hardening low-level primitives - all that 
> > a ring buffer implementation has to do is to store data in std::array from 
> > hardened libc++ and it's all set.
> 
> That doesn't help real world projects like LLVM where we replace the 
> low-level primitives when they're not appropriate for our domain. I think we 
> should assume that users will implement their own containers and support that 
> situation from the start.
Yes, so I think we want to eventually cover low-level primitives like 
`SmallVector`, and then ask people to implement their higher-level primitives 
like ring buffers in terms of the covered low-level primitives.

The situation when people really need to have ultra-optimized high-level 
primitives that are implemented directly with raw buffers, and then optionally 
harden that, becomes significantly more rare. In such cases maybe it's ok not 
to have a clear 

[PATCH] D137313: [NFC] Remove redundant loads when has_device_addr is used.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D137313#3906527 , @jyu2 wrote:

> In D137313#3906487 , @ABataev wrote:
>
>> Do we have a runtime test for this? Would be good to try to test it if the 
>> offloading fails and the host version is executed instead.
>
> I have runtime test in https://reviews.llvm.org/D134268
> openmp/libomptarget/test/mapping/target_has_device_addr.c where
>
> void bar() {
>
>   short x[10];
>   short *xp = [0];
>   
>   x[1] = 111;
>
> #pragma omp target data map(tofrom : xp [0:2]) use_device_addr(xp [0:2])
> #pragma omp target has_device_addr(xp [0:2])
>
>   {
> xp[1] = 222;
> // CHECK: 222
> printf("%d %p\n", xp[1], [1]);
>   }
>   // CHECK: 222
>   printf("%d %p\n", xp[1], [1]);
>
> }

Need to add a reference


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137313

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

[PATCH] D137058: [Driver] [Modules] Support -fsave-std-c++-module-file (1/2)

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a subscriber: rsmith.
dblaikie added a comment.

I realize I got this jumbled up and the thread about "why do we need to name 
things" is meant to be over in D137059  
(sorry @ben.boeckel :/ I know this is all confusing to follow at the best of 
times) so I'll pick that up there.

But maybe a relevant question /here/ (maybe @iains has some context here, and 
@ben.boeckel ): What are GCC's (& any other implementations) command line 
interfaces for things like this? How're the command line flags spelled? To see 
about inspiration for this.

(judging from the discourse thread with @rsmith it seems like "the ability to 
generate a .o from a .pcm" is disfavored, he didn't outright say "we should 
remove that functionality" but pretty close to it - in favor of ".cppm -> {.o, 
.pcm}" and ".ccpm -> .o" + ".cppm -> .pcm" (without the ability to generate the 
.o from the .pcm) - in which case we could change the existing driver behavior 
sooner or later to address that third action (.cppm -> .pcm but it's only a 
minimal pcm). Maybe we should have two flags one that says "produce a .pcm" 
(which we already have `--precompile` for that) and another that says "produce 
a .o" (I guess that's the default, so maybe we want a way to opt /out/ of that 
behavior?))

Eh, sorry, just talking myself around in circles.
Currently we have:
`.cppm` -> `.o` (no extra flags)
`.cppm` -> `.pcm` (`--precompile`)
`.cppm` -> `.pcm` + `.o` (unsupported)

I'm not sure that `--precompile` is the best flag name to be inspired by (it's 
unqualified by any `-f` or other prefix, which usually feels a bit weird, and 
it's a very generic term, doesn't mention modules, etc) - so I can appreciate 
the `-fsave-std-c++-module-file` name here, but it does sound a bit verbose? 
Wouldn't mind hearing other people's thoughts on flag names, especially any 
prior art/other implementation choices we could look for for inspiration.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137058

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

[PATCH] D137313: [NFC] Remove redundant loads when has_device_addr is used.

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 added a comment.

In D137313#3906487 , @ABataev wrote:

> Do we have a runtime test for this? Would be good to try to test it if the 
> offloading fails and the host version is executed instead.

I have runtime test in https://reviews.llvm.org/D134268
openmp/libomptarget/test/mapping/target_has_device_addr.c where

void bar() {

  short x[10];
  short *xp = [0];
  
  x[1] = 111;

#pragma omp target data map(tofrom : xp [0:2]) use_device_addr(xp [0:2])
#pragma omp target has_device_addr(xp [0:2])

  {
xp[1] = 222;
// CHECK: 222
printf("%d %p\n", xp[1], [1]);
  }
  // CHECK: 222
  printf("%d %p\n", xp[1], [1]);

}


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137313

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

[PATCH] D137224: clang/cmake: Simplify lit detection for standalone builds

[Tom Stellard via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc9a959334707: clang/cmake: Simplify lit detection for 
standalone builds (authored by tstellar).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137224

Files:
  clang/CMakeLists.txt


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -93,9 +93,14 @@
   set(LLVM_UTILS_PROVIDED ON)
 endif()
 
+# Seek installed Lit.
+find_program(LLVM_LIT
+ NAMES llvm-lit lit.py lit
+ PATHS "${LLVM_MAIN_SRC_DIR}/utils/lit"
+ DOC "Path to lit.py")
+
 if(EXISTS ${LLVM_MAIN_SRC_DIR}/utils/lit/lit.py)
   # Note: path not really used, except for checking if lit was found
-  set(LLVM_LIT ${LLVM_MAIN_SRC_DIR}/utils/lit/lit.py)
   if(EXISTS ${LLVM_MAIN_SRC_DIR}/utils/llvm-lit)
 add_subdirectory(${LLVM_MAIN_SRC_DIR}/utils/llvm-lit utils/llvm-lit)
   endif()
@@ -112,12 +117,6 @@
   AND EXISTS ${UNITTEST_DIR}/CMakeLists.txt)
 add_subdirectory(${UNITTEST_DIR} utils/unittest)
   endif()
-else()
-  # Seek installed Lit.
-  find_program(LLVM_LIT
-   NAMES llvm-lit lit.py lit
-   PATHS "${LLVM_MAIN_SRC_DIR}/utils/lit"
-   DOC "Path to lit.py")
 endif()
 
 if(LLVM_LIT)


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -93,9 +93,14 @@
   set(LLVM_UTILS_PROVIDED ON)
 endif()
 
+# Seek installed Lit.
+find_program(LLVM_LIT
+ NAMES llvm-lit lit.py lit
+ PATHS "${LLVM_MAIN_SRC_DIR}/utils/lit"
+ DOC "Path to lit.py")
+
 if(EXISTS ${LLVM_MAIN_SRC_DIR}/utils/lit/lit.py)
   # Note: path not really used, except for checking if lit was found
-  set(LLVM_LIT ${LLVM_MAIN_SRC_DIR}/utils/lit/lit.py)
   if(EXISTS ${LLVM_MAIN_SRC_DIR}/utils/llvm-lit)
 add_subdirectory(${LLVM_MAIN_SRC_DIR}/utils/llvm-lit utils/llvm-lit)
   endif()
@@ -112,12 +117,6 @@
   AND EXISTS ${UNITTEST_DIR}/CMakeLists.txt)
 add_subdirectory(${UNITTEST_DIR} utils/unittest)
   endif()
-else()
-  # Seek installed Lit.
-  find_program(LLVM_LIT
-   NAMES llvm-lit lit.py lit
-   PATHS "${LLVM_MAIN_SRC_DIR}/utils/lit"
-   DOC "Path to lit.py")
 endif()
 
 if(LLVM_LIT)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] c9a9593 - clang/cmake: Simplify lit detection for standalone builds

[Tom Stellard via cfe-commits]

Author: Tom Stellard
Date: 2022-11-03T15:12:55-07:00
New Revision: c9a959334707810795f7e8c56ab6dd55ff0a359d

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

LOG: clang/cmake: Simplify lit detection for standalone builds

Reviewed By: mgorny, phosek, Ericson2314

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

Added: 


Modified: 
clang/CMakeLists.txt

Removed: 




diff  --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt
index 236e6fbaca28..8763cc0c1caa 100644
--- a/clang/CMakeLists.txt
+++ b/clang/CMakeLists.txt
@@ -93,9 +93,14 @@ if(CLANG_BUILT_STANDALONE)
   set(LLVM_UTILS_PROVIDED ON)
 endif()
 
+# Seek installed Lit.
+find_program(LLVM_LIT
+ NAMES llvm-lit lit.py lit
+ PATHS "${LLVM_MAIN_SRC_DIR}/utils/lit"
+ DOC "Path to lit.py")
+
 if(EXISTS ${LLVM_MAIN_SRC_DIR}/utils/lit/lit.py)
   # Note: path not really used, except for checking if lit was found
-  set(LLVM_LIT ${LLVM_MAIN_SRC_DIR}/utils/lit/lit.py)
   if(EXISTS ${LLVM_MAIN_SRC_DIR}/utils/llvm-lit)
 add_subdirectory(${LLVM_MAIN_SRC_DIR}/utils/llvm-lit utils/llvm-lit)
   endif()
@@ -112,12 +117,6 @@ if(CLANG_BUILT_STANDALONE)
   AND EXISTS ${UNITTEST_DIR}/CMakeLists.txt)
 add_subdirectory(${UNITTEST_DIR} utils/unittest)
   endif()
-else()
-  # Seek installed Lit.
-  find_program(LLVM_LIT
-   NAMES llvm-lit lit.py lit
-   PATHS "${LLVM_MAIN_SRC_DIR}/utils/lit"
-   DOC "Path to lit.py")
 endif()
 
 if(LLVM_LIT)



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

[PATCH] D137313: [NFC] Remove redundant loads when has_device_addr is used.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

Do we have a runtime test for this? Would be good to try to test it if the 
offloading fails and the host version is executed instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137313

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

[PATCH] D137107: Allow MS extension: support of constexpr with __declspec(dllimport).

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D137107#3906439 , @zahiraam wrote:

> Here is an even smaller test case.
> lib.cpp:
> __declspec(dllexport) int val=12;
>
> t1.cpp:
> int main () {
>
>   extern int _declspec(dllimport) val;
>   int& val_ref = val;
>   return val;
>
> }
> $ clang.exe -LD lib.cpp (this will create a.lib and a.exp)
> $ clang.exe t1.cpp -la
> $ ./a.exe
> This will result in a popup message: "The application was unable to start 
> correctly ..."

What does `llvm-readobj --coff-imports a.exe` show here?

What environment are you running this in? `-LD` is a cl/clang-cl style option 
and can't be passed to the gnu-style interface of `clang.exe`.

When I try to follow these examples in cmd.exe, I get the following:

  >clang.exe -LD lib.cpp
 Creating library a.lib and object a.exp
  LINK : fatal error LNK1561: entry point must be defined
  clang: error: linker command failed with exit code 1561 (use -v to see 
invocation)

It looks to me like the way you're invoking the compiler here is creating the 
issue you're observing. This testcase works just fine for me when I build it 
like this:

  >clang-cl -LD lib.cpp
  >clang-cl t1.cpp -link lib.lib
  >t1.exe
  >echo %errorlevel%
  12


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137107

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

[Diffusion] rGfdab9f1203ee: [Clang] Check for response file existence prior to check for recursion

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a subscriber: cfe-commits.

BRANCHES
  main

Users:
  sepavloff (Author)

https://reviews.llvm.org/rGfdab9f1203ee

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

[PATCH] D137259: [clang][modules][deps] WIP: In-memory module transfer

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 473043.
jansvoboda11 added a comment.

Handle macros


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137259

Files:
  clang/include/clang/Frontend/CompilerInstance.h
  clang/include/clang/Lex/Token.h
  clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Serialization/GeneratePCH.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
  clang/test/ClangScanDeps/modules-basic.c

Index: clang/test/ClangScanDeps/modules-basic.c
===
--- /dev/null
+++ clang/test/ClangScanDeps/modules-basic.c
@@ -0,0 +1,76 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+//--- include/module.modulemap
+module m { header "m.h"}
+//--- include/m.h
+#define FOO
+//--- include/t.h
+
+//--- tu.c
+#include "m.h"
+#ifdef FOO
+#include "t.h"
+#endif
+
+//--- cdb.json.template
+[{
+  "file": "DIR/tu.c",
+  "directory": "DIR",
+  "command": "clang -fsyntax-only DIR/tu.c -fmodules -fmodules-cache-path=DIR/cache -fimplicit-module-maps -I DIR/include"
+}]
+
+// RUN: sed "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json
+
+// RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full > %t/result.json
+// RUN: cat %t/result.json | sed 's:\?:/:g' | FileCheck %s -DPREFIX=%/t
+
+// CHECK: {
+// CHECK-NEXT:   "modules": [
+// CHECK-NEXT: {
+// CHECK-NEXT:   "clang-module-deps": [],
+// CHECK-NEXT:   "clang-modulemap-file": "[[PREFIX]]/include/module.modulemap",
+// CHECK-NEXT:   "command-line": [
+// CHECK-NEXT: "-cc1",
+// CHECK:  "-o",
+// CHECK-NEXT: "[[PREFIX]]/cache/{{.*}}/m-{{.*}}.pcm",
+// CHECK:  "-emit-module",
+// CHECK:  "[[PREFIX]]/include/module.modulemap",
+// CHECK:  "-fmodules",
+// CHECK:  "-fmodule-name=m",
+// CHECK:],
+// CHECK-NEXT:   "context-hash": "[[M_HASH:.*]]",
+// CHECK-NEXT:   "file-deps": [
+// CHECK-NEXT: "[[PREFIX]]/include/m.h",
+// CHECK-NEXT: "[[PREFIX]]/include/module.modulemap"
+// CHECK-NEXT:   ],
+// CHECK-NEXT:   "name": "m"
+// CHECK-NEXT: }
+// CHECK-NEXT:   ],
+// CHECK-NEXT:   "translation-units": [
+// CHECK-NEXT: {
+// CHECK-NEXT:   "commands": [
+// CHECK-NEXT: {
+// CHECK-NEXT:   "clang-context-hash": "{{.*}}",
+// CHECK-NEXT:   "clang-module-deps": [
+// CHECK-NEXT: {
+// CHECK-NEXT:   "context-hash": "[[M_HASH]]",
+// CHECK-NEXT:   "module-name": "m"
+// CHECK-NEXT: }
+// CHECK-NEXT:   ],
+// CHECK-NEXT:   "command-line": [
+// CHECK-NEXT: "-cc1",
+// CHECK:  "-fmodule-map-file=[[PREFIX]]/include/module.modulemap",
+// CHECK:  "-fmodule-file=m=[[PREFIX]]/cache/{{.*}}/m-{{.*}}.pcm",
+// CHECK:],
+// CHECK-NEXT:   "executable": "clang",
+// CHECK-NEXT:   "file-deps": [
+// CHECK-NEXT: "[[PREFIX]]/tu.c",
+// CHECK-NEXT: "[[PREFIX]]/include/t.h"
+// CHECK-NEXT:   ],
+// CHECK-NEXT:   "input-file": "[[PREFIX]]/tu.c"
+// CHECK-NEXT: }
+// CHECK-NEXT:   ]
+// CHECK-NEXT: }
+// CHECK-NEXT:   ]
+// CHECK-NEXT: }
Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -375,15 +375,8 @@
 if (!MDC.isPrebuiltModule(M))
   DirectModularDeps.insert(M);
 
-  for (const Module *M : DirectModularDeps) {
-// A top-level module might not be actually imported as a module when
-// -fmodule-name is used to compile a translation unit that imports this
-// module. In that case it can be skipped. The appropriate header
-// dependencies will still be reported as expected.
-if (!M->getASTFile())
-  continue;
+  for (const Module *M : DirectModularDeps)
 handleTopLevelModule(M);
-  }
 
   MDC.Consumer.handleDependencyOutputOpts(*MDC.Opts);
 
@@ -410,7 +403,6 @@
 
   MD.ID.ModuleName = M->getFullModuleName();
   MD.ImportedByMainFile = DirectModularDeps.contains(M);
-  MD.ImplicitModulePCMPath = std::string(M->getASTFile()->getName());
   MD.IsSystem = M->IsSystem;
 
   ModuleMap  =
@@ -424,40 +416,30 @@
 MD.ClangModuleMapFile = std::string(Path);
   }
 
-  serialization::ModuleFile *MF =
-  MDC.ScanInstance.getASTReader()->getModuleManager().lookup(
-  M->getASTFile());
-  MDC.ScanInstance.getASTReader()->visitInputFiles(
-  *MF, true, true, [&](const serialization::InputFile , bool isSystem) {
-// __inferred_module.map is the result of the way in which an implicit
-// module build handles 

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

[Noah Goldstein via Phabricator via cfe-commits]

goldstein.w.n added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:5056-5059
+   "if (A) { \\\n"
+   "B(); \\\n"
+   "}\\\n"
+   "C();\n"

goldstein.w.n wrote:
> owenpan wrote:
> > goldstein.w.n wrote:
> > > owenpan wrote:
> > > > I just noticed that here and below you got an extra `IndentWidth` than 
> > > > in the summary, so the patch only works for `PPDIS_None`?
> > > To a degree.
> > > 
> > > `Level` is used by both scope depth and PP depth so nested PP directives 
> > > with before/after will essentially have `IndentWidth * (PPLevel + 
> > > ScopeLevel)` as net indentation.
> > > 
> > > AFAICT `UnwrappedLineParser.cpp::parsePPDefine` needs to something other 
> > > than `Line->Level` with `PBBranchLevel + 1`.
> > > 
> > > I started doing that but the diff got a bit bigger given that it needs to 
> > > get propegated between Wrapper/Unwrapped/Annotated (AFAICT).
> > > 
> > > Would you prefer its implemented like that?
> > See D137181#3904383
> Hmm? What is that suppose to link to?
Err, sorry I see now. Got it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137181

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

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

[Noah Goldstein via Phabricator via cfe-commits]

goldstein.w.n added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:5056-5059
+   "if (A) { \\\n"
+   "B(); \\\n"
+   "}\\\n"
+   "C();\n"

owenpan wrote:
> goldstein.w.n wrote:
> > owenpan wrote:
> > > I just noticed that here and below you got an extra `IndentWidth` than in 
> > > the summary, so the patch only works for `PPDIS_None`?
> > To a degree.
> > 
> > `Level` is used by both scope depth and PP depth so nested PP directives 
> > with before/after will essentially have `IndentWidth * (PPLevel + 
> > ScopeLevel)` as net indentation.
> > 
> > AFAICT `UnwrappedLineParser.cpp::parsePPDefine` needs to something other 
> > than `Line->Level` with `PBBranchLevel + 1`.
> > 
> > I started doing that but the diff got a bit bigger given that it needs to 
> > get propegated between Wrapper/Unwrapped/Annotated (AFAICT).
> > 
> > Would you prefer its implemented like that?
> See D137181#3904383
Hmm? What is that suppose to link to?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137181

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

[PATCH] D137107: Allow MS extension: support of constexpr with __declspec(dllimport).

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added a comment.

In D137107#3906413 , @mstorsjo wrote:

> In D137107#3905443 , @zahiraam 
> wrote:
>
>> and the same assembly (load __imp_?val) (is that why you made the comment 
>> above?). And the test case runs.
>
> Yes, pretty much - the actual value of the address of `` isn't known at 
> compile time and it cannot be produced in a way to initialize e.g. static 
> memory, which I would believe that one can expect of constexpr. The runtime 
> linker that loads DLLs can't fix it so that the address is available as a 
> statically initialized data, but to use it it does require runtime code to 
> fetch the address.
>
>> Without  constexpr the symbol generated is
>>
>> 0E  UNDEF  notype   External | __imp_?val@@3HA 
>> (__declspec(dllimport) int val)
>>
>> and the assembly generates a ‘load __imp_? Val’. Same than MSVC, but the 
>> test case fails to run.  @rnk Shouldn't this run?
>
> Can you give the full exact repro for this case, without constexpr, which you 
> say fails to run - I tried to follow the instructions but for me it seems to 
> work just fine, without constexpr.

Here is an even smaller test case.
lib.cpp:
__declspec(dllexport) int val=12;

t1.cpp:
int main () {

  extern int _declspec(dllimport) val;
  int& val_ref = val;
  return val;

}
$ clang.exe -LD lib.cpp (this will create a.lib and a.exp)
$ clang.exe t1.cpp -la
$ ./a.exe
This will result in a popup message: "The application was unable to start 
correctly ..."

With MSVC
$ cl /LD lib.cpp (this will create lib.lib and lib.dll)
$ cl t1.cpp /link lib.lib
$ t1.exe
$ echo $?
12
$


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137107

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

[PATCH] D137327: [clang-format] Handle object instansiation in if-statements

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

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:366
+  CurrentToken->is(tok::identifier) &&
+  !Next->isOneOf(tok::equal, tok::l_brace)) {
 Prev->setType(TT_BinaryOperator);

Could you add a test for this as well?



Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:1038
 
+TEST_F(TokenAnnotatorTest, UnderstandIfInitializer) {
+  auto Tokens = annotate("if (Class* obj {getObj()}) ");

There is already a test case for star amp understanding, please use that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137327

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

[PATCH] D137107: Allow MS extension: support of constexpr with __declspec(dllimport).

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D137107#3905443 , @zahiraam wrote:

> and the same assembly (load __imp_?val) (is that why you made the comment 
> above?). And the test case runs.

Yes, pretty much - the actual value of the address of `` isn't known at 
compile time and it cannot be produced in a way to initialize e.g. static 
memory, which I would believe that one can expect of constexpr. The runtime 
linker that loads DLLs can't fix it so that the address is available as a 
statically initialized data, but to use it it does require runtime code to 
fetch the address.

> Without  constexpr the symbol generated is
>
> 0E  UNDEF  notype   External | __imp_?val@@3HA 
> (__declspec(dllimport) int val)
>
> and the assembly generates a ‘load __imp_? Val’. Same than MSVC, but the test 
> case fails to run.  @rnk Shouldn't this run?

Can you give the full exact repro for this case, without constexpr, which you 
say fails to run - I tried to follow the instructions but for me it seems to 
work just fine, without constexpr.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137107

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

[PATCH] D137313: [NFC] Remove redundant loads when has_device_addr is used.

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 added a comment.

In D137313#3904805 , @ABataev wrote:

> Whyt it is NFC? I assume it fixes the dereferencing of references, right?

Thanks  Alexey.

I really don't expect run time change for this change.  The change only change 
BP/PP of the map instead I was using

  CGF.EmitLValue(E).getPointer(CGF);

or

  CGF.EmitScalarExpr(E);

to generate BP/PP.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137313

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

[PATCH] D136796: [llvm-objdump][Offload] Use common offload extraction method

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added inline comments.



Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:1262-1265
+if (identify_magic((*BufferOrErr)->getBuffer()) ==
+file_magic::elf_shared_object)
+  continue;
+

tra wrote:
> jhenderson wrote:
> > This change seems not really part of the patch, since it's touching `clang` 
> > but the patch is for `llvm-objdump`.
> +1 to landing this separately, possibly with a relevant test.
I put it in a separate commit but it seems arcanist squashed it in the 
background.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136796

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

[clang] 3384f05 - [llvm-objdump][Offload] Use common offload extraction method

[Joseph Huber via cfe-commits]

Author: Joseph Huber
Date: 2022-11-03T16:19:13-05:00
New Revision: 3384f05a2cdb96a2f106c234ae8a9d0e306717a4

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

LOG: [llvm-objdump][Offload] Use common offload extraction method

A previous patch introduced a common function used to extract offloading
binaries from an image. Therefore we no longer need to duplicate the
functionality in the `llvm-objdump` implementation. Functionally, this
removes the old warning behaviour when given malformed input. This has
been changed to a hard error, which is effectively the same.

This required a slight tweak in the linker wrapper to filter out the
user passing shared objects directly.

Reviewed By: tra

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

Added: 
llvm/test/tools/llvm-objdump/Offloading/elf.test

Modified: 
clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
llvm/lib/Object/OffloadBinary.cpp
llvm/test/tools/llvm-objdump/Offloading/content-failure.test
llvm/tools/llvm-objdump/OffloadDump.cpp

Removed: 
llvm/test/tools/llvm-objdump/Offloading/binary.test
llvm/test/tools/llvm-objdump/Offloading/warning.test



diff  --git a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp 
b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
index 3ad22be755f3c..6a12b64f7d7dd 100644
--- a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -1259,6 +1259,10 @@ Expected> getDeviceInput(const 
ArgList ) {
 if (std::error_code EC = BufferOrErr.getError())
   return createFileError(Filename, EC);
 
+if (identify_magic((*BufferOrErr)->getBuffer()) ==
+file_magic::elf_shared_object)
+  continue;
+
 bool IsLazy =
 identify_magic((*BufferOrErr)->getBuffer()) == file_magic::archive;
 if (Error Err = extractOffloadBinaries(

diff  --git a/llvm/lib/Object/OffloadBinary.cpp 
b/llvm/lib/Object/OffloadBinary.cpp
index 8f62d692d050a..3f7a60d89c27d 100644
--- a/llvm/lib/Object/OffloadBinary.cpp
+++ b/llvm/lib/Object/OffloadBinary.cpp
@@ -41,6 +41,10 @@ Error extractOffloadFiles(MemoryBufferRef Contents,
 std::unique_ptr Buffer =
 MemoryBuffer::getMemBuffer(Contents.getBuffer().drop_front(Offset), "",
/*RequiresNullTerminator*/ false);
+if (!isAddrAligned(Align(OffloadBinary::getAlignment()),
+   Buffer->getBufferStart()))
+  Buffer = MemoryBuffer::getMemBufferCopy(Buffer->getBuffer(),
+  Buffer->getBufferIdentifier());
 auto BinaryOrErr = OffloadBinary::create(*Buffer);
 if (!BinaryOrErr)
   return BinaryOrErr.takeError();
@@ -254,7 +258,9 @@ Error object::extractOffloadBinaries(MemoryBufferRef Buffer,
   switch (Type) {
   case file_magic::bitcode:
 return extractFromBitcode(Buffer, Binaries);
-  case file_magic::elf_relocatable: {
+  case file_magic::elf_relocatable:
+  case file_magic::elf_executable:
+  case file_magic::elf_shared_object: {
 Expected> ObjFile =
 ObjectFile::createObjectFile(Buffer, Type);
 if (!ObjFile)

diff  --git a/llvm/test/tools/llvm-objdump/Offloading/content-failure.test 
b/llvm/test/tools/llvm-objdump/Offloading/content-failure.test
index 5089edae04502..40ff6785f2d38 100644
--- a/llvm/test/tools/llvm-objdump/Offloading/content-failure.test
+++ b/llvm/test/tools/llvm-objdump/Offloading/content-failure.test
@@ -15,4 +15,4 @@ Sections:
 ShOffset:0x9
 AddressAlign:0x0008
 
-# CHECK: error: '[[FILENAME]]': The end of the file was unexpectedly 
encountered
+# CHECK: error: '[[FILENAME]]': while extracting offloading files: The end of 
the file was unexpectedly encountered

diff  --git a/llvm/test/tools/llvm-objdump/Offloading/binary.test 
b/llvm/test/tools/llvm-objdump/Offloading/elf.test
similarity index 67%
rename from llvm/test/tools/llvm-objdump/Offloading/binary.test
rename to llvm/test/tools/llvm-objdump/Offloading/elf.test
index 880bab2ec5337..10182aeb856cd 100644
--- a/llvm/test/tools/llvm-objdump/Offloading/binary.test
+++ b/llvm/test/tools/llvm-objdump/Offloading/elf.test
@@ -3,15 +3,21 @@
 # RUN: llvm-objdump --offloading %t.bin | FileCheck %s --match-full-lines 
--strict-whitespace --implicit-check-not={{.}}
 
 ## Check that we can dump an offloading binary inside of an ELF section.
-# RUN: yaml2obj %s -o %t.elf
-# RUN: llvm-objcopy --update-section .llvm.offloading=%t.bin %t.elf
-# RUN: llvm-objdump --offloading %t.elf | FileCheck %s 
--check-prefixes=CHECK,ELF --match-full-lines --strict-whitespace 
--implicit-check-not={{.}}
+# RUN: yaml2obj %s -o %t -DTYPE=ET_EXEC
+# RUN: yaml2obj %s -o %t.so -DTYPE=ET_DYN
+# RUN: yaml2obj %s -o %t.o -DTYPE=ET_REL

[PATCH] D136796: [llvm-objdump][Offload] Use common offload extraction method

[Joseph Huber 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 rG3384f05a2cdb: [llvm-objdump][Offload] Use common offload 
extraction method (authored by jhuber6).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136796

Files:
  clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
  llvm/lib/Object/OffloadBinary.cpp
  llvm/test/tools/llvm-objdump/Offloading/binary.test
  llvm/test/tools/llvm-objdump/Offloading/content-failure.test
  llvm/test/tools/llvm-objdump/Offloading/elf.test
  llvm/test/tools/llvm-objdump/Offloading/warning.test
  llvm/tools/llvm-objdump/OffloadDump.cpp

Index: llvm/tools/llvm-objdump/OffloadDump.cpp
===
--- llvm/tools/llvm-objdump/OffloadDump.cpp
+++ llvm/tools/llvm-objdump/OffloadDump.cpp
@@ -10,6 +10,7 @@
 /// This file implements the offloading-specific dumper for llvm-objdump.
 ///
 //===--===//
+
 #include "OffloadDump.h"
 #include "llvm-objdump.h"
 #include "llvm/Object/ELFObjectFile.h"
@@ -46,24 +47,6 @@
  << getOffloadKindName(OB.getOffloadKind()) << "\n";
 }
 
-static Error visitAllBinaries(const OffloadBinary ) {
-  uint64_t Offset = 0;
-  uint64_t Index = 0;
-  while (Offset < OB.getMemoryBufferRef().getBufferSize()) {
-MemoryBufferRef Buffer =
-MemoryBufferRef(OB.getData().drop_front(Offset), OB.getFileName());
-auto BinaryOrErr = OffloadBinary::create(Buffer);
-if (!BinaryOrErr)
-  return BinaryOrErr.takeError();
-
-OffloadBinary  = **BinaryOrErr;
-printBinary(Binary, Index++);
-
-Offset += Binary.getSize();
-  }
-  return Error::success();
-}
-
 /// Print the embedded offloading contents of an ObjectFile \p O.
 void llvm::dumpOffloadBinary(const ObjectFile ) {
   if (!O.isELF()) {
@@ -72,41 +55,25 @@
 return;
   }
 
-  for (ELFSectionRef Sec : O.sections()) {
-if (Sec.getType() != ELF::SHT_LLVM_OFFLOADING)
-  continue;
-
-Expected Contents = Sec.getContents();
-if (!Contents)
-  reportError(Contents.takeError(), O.getFileName());
-
-std::unique_ptr Buffer =
-MemoryBuffer::getMemBuffer(*Contents, O.getFileName(), false);
-if (!isAddrAligned(Align(OffloadBinary::getAlignment()),
-   Buffer->getBufferStart()))
-  Buffer = MemoryBuffer::getMemBufferCopy(Buffer->getBuffer(),
-  Buffer->getBufferIdentifier());
-auto BinaryOrErr = OffloadBinary::create(*Buffer);
-if (!BinaryOrErr)
-  reportError(O.getFileName(), "while extracting offloading files: " +
-   toString(BinaryOrErr.takeError()));
-OffloadBinary  = **BinaryOrErr;
+  SmallVector Binaries;
+  if (Error Err = extractOffloadBinaries(O.getMemoryBufferRef(), Binaries))
+reportError(O.getFileName(), "while extracting offloading files: " +
+ toString(std::move(Err)));
 
-// Print out all the binaries that are contained in this buffer. If we fail
-// to parse a binary before reaching the end of the buffer emit a warning.
-if (Error Err = visitAllBinaries(Binary))
-  reportWarning("while parsing offloading files: " +
-toString(std::move(Err)),
-O.getFileName());
-  }
+  // Print out all the binaries that are contained in this buffer.
+  for (uint64_t I = 0, E = Binaries.size(); I != E; ++I)
+printBinary(*Binaries[I].getBinary(), I);
 }
 
 /// Print the contents of an offload binary file \p OB. This may contain
 /// multiple binaries stored in the same buffer.
 void llvm::dumpOffloadSections(const OffloadBinary ) {
-  // Print out all the binaries that are contained at this buffer. If we fail to
-  // parse a binary before reaching the end of the buffer emit a warning.
-  if (Error Err = visitAllBinaries(OB))
-reportWarning("while parsing offloading files: " + toString(std::move(Err)),
-  OB.getFileName());
+  SmallVector Binaries;
+  if (Error Err = extractOffloadBinaries(OB.getMemoryBufferRef(), Binaries))
+reportError(OB.getFileName(), "while extracting offloading files: " +
+  toString(std::move(Err)));
+
+  // Print out all the binaries that are contained in this buffer.
+  for (uint64_t I = 0, E = Binaries.size(); I != E; ++I)
+printBinary(*Binaries[I].getBinary(), I);
 }
Index: llvm/test/tools/llvm-objdump/Offloading/warning.test
===
--- llvm/test/tools/llvm-objdump/Offloading/warning.test
+++ /dev/null
@@ -1,21 +0,0 @@
-## Ensure we give a warning on bad input following good input.
-# RUN: yaml2obj %S/Inputs/binary.yaml -o %t-good.bin
-# RUN: yaml2obj %S/Inputs/malformed.yaml -o %t-bad.bin
-# 

[PATCH] D136815: [clang][Interp] Unify visiting variable declarations

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.h:282
+  bool isGlobalDecl(const VarDecl *VD) const {
+return !VD->hasLocalStorage() || VD->isConstexpr();
+  }

tbaeder wrote:
> shafik wrote:
> > tbaeder wrote:
> > > shafik wrote:
> > > > Why not `hasGlobalStorage()`?
> > > > 
> > > > Also what is the logic behind `isConstexpr()` check?
> > > Didn't know `isGlobalStorage()` existed ;)
> > > 
> > > Constexpr local variables can be handled like global ones, can't they? 
> > > That was the logic behind it, nothing else. We can save ourselves the 
> > > hassle of local variables in that case.
> > I think I am missing a level of logic here. I don't think of constant 
> > expressions as needing storage nor do I think of them as global variables.
> > 
> > So can you take a step back and explain how this fits in the bigger picture?
> They don't necessarily need storage in the final executable but we create 
> global/local variables for them for bookkeeping, e.g. we need to be able to 
> take their address, track the value, etc.
Ok, will this work for recursive `constexpr` functions with local variables?


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

https://reviews.llvm.org/D136815

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

[PATCH] D136894: Add clang-doc readme

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added a comment.

In D136894#3905638 , @brettw wrote:

> I definitely wasn't intending to write "formal documentation" for clang-doc. 
> I was trying to write some notes for a future contributor as I'm winding down 
> my work here, and it seems nobody else is working on it or has any knowledge 
> about it. Is there a place in LLVM for this type of documentation?

Most of LLVM's documentation is either in the formal documentation. Many things 
are also only documented in the code, though. We're trying to be better about 
that these days, but manyt things are only described in the comments of a 
particular `.cpp` file.

> I can rename the file to "notes" or something if you think that would help 
> make things more clear.

I can't think of anything like that elsewhere in our code base. If you don't 
want formal documentation, you can take the parts you think are appropriate and 
add them as comments to the relevant files. For things you think should be 
changed, an issue on github is probably the best way to make sure that gets 
addressed, and reference that somewhere in clang-doc's code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136894

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

[PATCH] D136811: -Wunsafe-buffer-usage: WIP: RFC: NFC: User documentation.

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: clang/docs/SafeBuffers.rst:31
+convert large amounts of old code to conform to the warning;
+  - Attribute ``[[unsafe_buffer_usage]]`` lets you annotate custom functions as
+unsafe, while providing a safe alternative that can often be suggested by

aaron.ballman wrote:
> xazax.hun wrote:
> > aaron.ballman wrote:
> > > H that attribute name looks problematic (more below in that section).
> > With C++ modules gaining some momentum, I wonder if it would be 
> > possible/beneficial to be able to annotate module import/export 
> > declarations with this attribute to signal that the APIs and the content in 
> > a module was not yet hardened. 
> I think it makes sense on module export (the module author knows whether they 
> are buffer-safe or not), but what do you have in mind for import where the 
> user may not be in a position to make that assertion validly?
Wouldn't `only_safe_buffers` make sense on the import side to ensure that no 
unhardened code is being imported from a specific module?



Comment at: clang/docs/SafeBuffers.rst:40-41
+  - Finally, in order to avoid bugs in newly converted code, the
+Clang static analyzer provides a checker to find misconstructed
+``std::span`` objects.
+

jkorous wrote:
> xazax.hun wrote:
> > Isn't this overly specific? What about `string_view`s? 
> This is based on our current plans for near future.
> While we are positive we will use `std::span` in the FixIts it is very 
> unlikely we'd use `string_view`. That's why having a checker for `std::span` 
> is more important for us now.
Does this mean you'd recommend using a `span` over `string_view` in all 
cases? I think that might surprise some users, so I wonder if it is worth 
documenting. 



Comment at: clang/docs/SafeBuffers.rst:287-288
+
+The automatic fixit associated with the warning would transform this array
+into an ``std::array``:
+

jkorous wrote:
> xazax.hun wrote:
> > I anticipate that doing these fixits for multi-dimensional arrays will be 
> > confusing. Is that in scope?
> Do you mean that the dimensions get reversed if we replace `int 
> arr_3d[3][4][5]` with `array, 4>,  3> arr_3d_r`?
Yeah, I can imagine some people wanting bounds safety but having strong 
opinions about the reversed dimensions in their code. Moreover, if there are 
multi-dimensional array typedefs in the nesting, that can make things more 
complicated.



Comment at: clang/docs/SafeBuffers.rst:324-326
+and binary compatibility. In order to avoid that, the fix will provide
+a compatibility overload to preserve the old functionality. The updated code
+produced by the fixit will look like this:

jkorous wrote:
> xazax.hun wrote:
> > While this might be desirable for some projects, I can imagine other 
> > projects want to avoid generating the overload (just generate the fixes in 
> > one pass, and apply all of them in a second to avoid a "fixed" header 
> > render another TU invalid). But that would require fixits for the call 
> > sites. Do you plan to support those batch migration scenarios or is that 
> > out of scope?
> We currently don't have a machinery that would allow us to analyze the whole 
> project at once.
> 
> Two passes are generally not enough - that number is something like number of 
> unique functions in the deepest call-stack.
> 
> Part of our fixits is attributing functions as unsafe and application of such 
> fixits can create new warnings in callers.
> 
> Let's take an example:
> 
> ```
> foo(int* ptr) {
>   ptr[5] = 5;
> }
> 
> bar(int* ptr) {
>   foo(ptr);
> }
> 
> baz(int* ptr) {
>   bar(ptr);
> }
> ```
> 
> Initially it is not obvious that `bar` or `baz` might get transformed.
> 
> In the first iteration a fixit for `foo` is emitted and when applied we get:
> ```
> foo(span ptr) {
>   ptr[5] = 5;
> }
> 
> [[clang::unsafe_buffer_usage]] foo(int* ptr);
> 
> bar(int* ptr) {
>   foo(ptr);
> }
> 
> baz(int* ptr) {
>   bar(ptr);
> }
> ```
> 
> Only now we learn that `bar` (which can live in a diferent TU) should get 
> transformed as well because it calls to unsafe function `foo`. But our 
> analysis still won't see that `baz` will eventually have to be transformed 
> too and it'll take another iteration.
I see, this explains it all, thanks! I'd love to have this motivating example 
in the design document why a 2-pass fix-it-all model does not work.



Comment at: clang/docs/SafeBuffers.rst:354-357
+  - Despite accepting a ``std::span`` which carries size information,
+the fixed function still accepts ``size`` separately. It can be removed
+manually, or it can be preserved, if ``size`` and ``buf.size()``
+actually need to be different in your case.

jkorous wrote:
> xazax.hun wrote:
> > I see this part a bit confusing. While it is true that we cannot be sure 
> > whether the size 

[PATCH] D137302: [clang-tidy] Add modernize-type-traits check

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 473028.
njames93 marked 3 inline comments as done.
njames93 added a comment.

Address documentation comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137302

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.cpp
  clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize/type-traits.rst
  clang-tools-extra/test/clang-tidy/checkers/modernize/type-traits.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize/type-traits.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/type-traits.cpp
@@ -0,0 +1,62 @@
+// RUN: %check_clang_tidy -std=c++14 %s modernize-type-traits %t
+// RUN: %check_clang_tidy -std=c++17 %s modernize-type-traits %t -check-suffixes=',CXX17'
+
+namespace std{
+  template 
+  struct is_const {
+static constexpr bool value = true;
+  };
+
+  template 
+  struct is_same {
+static constexpr bool value = true;
+  };
+
+  template
+  struct enable_if {
+using type = T;
+  };
+} // namespace std
+
+bool NoTemplate = std::is_const::value;
+// CHECK-MESSAGES-CXX17: :[[@LINE-1]]:19: warning: use c++17 style variable templates
+// CHeCK-FIXES-CXX17: bool NoTemplate = std::is_const_v
+
+template
+constexpr bool InTemplate = std::is_const::value;
+// CHECK-MESSAGES-CXX17: :[[@LINE-1]]:29: warning: use c++17 style variable templates
+// CHECK-FIXES-CXX17: constexpr bool InTemplate = std::is_const_v;
+
+template
+constexpr bool Template2Params = std::is_same::value;
+// CHECK-MESSAGES-CXX17: :[[@LINE-1]]:34: warning: use c++17 style variable templates
+// CHECK-FIXES-CXX17: constexpr bool Template2Params = std::is_same_v;
+
+template
+typename std::enable_if::type inTemplate();
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use c++14 style type templates
+// CHECK-FIXES: std::enable_if_tinTemplate();
+
+typename std::enable_if::type noTemplate();
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use c++14 style type templates
+// CHECK-FIXES: std::enable_if_tnoTemplate();
+
+std::enable_if::type noTemplateOrTypename();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use c++14 style type templates
+// CHECK-FIXES: std::enable_if_tnoTemplateOrTypename();
+
+using UsingNoTypename = std::enable_if::type;
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use c++14 style type templates
+// CHECK-FIXES: using UsingNoTypename = std::enable_if_t;
+
+// For macros we don't want any fixes to be emitted
+
+#define VALUE_MACRO std::is_same::value
+bool MacroValue = VALUE_MACRO;
+// CHECK-MESSAGES-CXX17: :[[@LINE-1]]:19: warning: use c++17 style variable templates
+// CHECK-FIXES-CXX17: #define VALUE_MACRO std::is_same::value
+
+#define TYPE_MACRO typename std::enable_if::type
+using MacroType = TYPE_MACRO;
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use c++14 style type templates
+// CHECK-FIXES: #define TYPE_MACRO typename std::enable_if::type
Index: clang-tools-extra/docs/clang-tidy/checks/modernize/type-traits.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/modernize/type-traits.rst
@@ -0,0 +1,25 @@
+.. title:: clang-tidy - modernize-type-traits
+
+modernize-type-traits
+=
+
+Converts standard library type traits of the form ``traits<...>::type`` and
+``traits<...>::value`` into ``traits_t<...>`` and ``traits_v<...>`` respectively.
+
+For example:
+
+.. code-block:: c++
+
+  std::is_integral::value
+  std::is_same::value
+  typename std::add_const::type
+  std::make_signed::type
+
+Would be converted into:
+
+.. code-block:: c++
+
+  std::is_integral_v
+  std::is_same_v
+  std::add_const_t
+  std::make_signed_t
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
@@ -276,6 +276,7 @@
`modernize-replace-random-shuffle `_, "Yes"
`modernize-return-braced-init-list `_, "Yes"
`modernize-shrink-to-fit `_, "Yes"
+   `modernize-type-traits `_, "Yes"
`modernize-unary-static-assert `_, "Yes"
`modernize-use-auto `_, "Yes"
`modernize-use-bool-literals `_, "Yes"
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -115,6 +115,12 @@
 
   Warns when using ``do-while`` loops.
 
+- New :doc:`modernize-type-traits
+  ` check.
+
+  Converts standard library type traits of the 

[PATCH] D137350: [RISCV] Implement assembler support for XVentanaCondOps

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

In D137350#3906234 , @reames wrote:

> In D137350#3906126 , @craig.topper 
> wrote:
>
>> Do these need their own DecoderNameSpace?
>
> What is a decoder namespace?  Some quick grepping and googling isn't very 
> informative.

If another vendor also uses these opcodes for something else, I think we would 
need to partition the disassembler tables.

Some existing examples

  RISCVInstrInfoC.td
  330:let DecoderNamespace = "RISCV32Only_",
  364:let DecoderNamespace = "RISCV32Only_",
  409:DecoderNamespace = "RISCV32Only_", Defs = [X1],
  517:let DecoderNamespace = "RISCV32Only_",
  577:let DecoderNamespace = "RISCV32Only_",

`RISCVDisassembler::getInstruction` checks some subtarget bits to select 
different tables where there are conflicts.


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

https://reviews.llvm.org/D137350

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

[PATCH] D137340: [clang-tidy] Add modernize-use-anonymous-namespace check

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

In D137340#3906250 , @njames93 wrote:

> I'm not entirely sure why this belongs in the modernize module given 
> anonymous namespaces have been in c++ forever, maybe its more of a misc 
> check? Also the modernize checks are meant to actually emit fixes(ignore the 
> c arrays check :) ), Right now, this only warn, it doesn't appear to act

Don't have a strong opinion, if you want I can move it there! I put in 
modernize as in "modernize from C-style into C++ style". But I know there's 
diverse opinions about this (LLVM not following this, for example) so perhaps 
it should be misc. Let me know which one you'd prefer so I can apply the change!

I didn't add fix because it would add quite some complexity, e.g. figuring out 
if there's is already an anon namespace to move into, or create a new one, 
users might want configurable behavior, etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137340

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

[PATCH] D136712: Define _GNU_SOURCE for arm baremetal in C++ mode.

[Manoj Gupta via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2497d5aa7716: Define _GNU_SOURCE for arm baremetal in C++ 
mode. (authored by manojgupta).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136712

Files:
  clang/lib/Basic/Targets/ARM.cpp
  clang/test/Preprocessor/init-arm.c


Index: clang/test/Preprocessor/init-arm.c
===
--- clang/test/Preprocessor/init-arm.c
+++ clang/test/Preprocessor/init-arm.c
@@ -1450,3 +1450,8 @@
 
 // THUMB-MINGW:#define __ARM_DWARF_EH__ 1
 
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=thumbv6m-none-unknown-eabi < 
/dev/null | FileCheck -match-full-lines -check-prefix Thumbv6m-elf %s
+// Thumbv6m-elf: #define __ELF__ 1
+
+// RUN: %clang_cc1 -x c++ -E -dM -ffreestanding 
-triple=thumbv6m-none-unknown-eabi < /dev/null | FileCheck -match-full-lines 
-check-prefix Thumbv6m-cxx %s
+// Thumbv6m-cxx: #define _GNU_SOURCE 1
Index: clang/lib/Basic/Targets/ARM.cpp
===
--- clang/lib/Basic/Targets/ARM.cpp
+++ clang/lib/Basic/Targets/ARM.cpp
@@ -705,8 +705,11 @@
   // For bare-metal none-eabi.
   if (getTriple().getOS() == llvm::Triple::UnknownOS &&
   (getTriple().getEnvironment() == llvm::Triple::EABI ||
-   getTriple().getEnvironment() == llvm::Triple::EABIHF))
+   getTriple().getEnvironment() == llvm::Triple::EABIHF)) {
 Builder.defineMacro("__ELF__");
+if (Opts.CPlusPlus)
+  Builder.defineMacro("_GNU_SOURCE");
+  }
 
   // Target properties.
   Builder.defineMacro("__REGISTER_PREFIX__", "");


Index: clang/test/Preprocessor/init-arm.c
===
--- clang/test/Preprocessor/init-arm.c
+++ clang/test/Preprocessor/init-arm.c
@@ -1450,3 +1450,8 @@
 
 // THUMB-MINGW:#define __ARM_DWARF_EH__ 1
 
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=thumbv6m-none-unknown-eabi < /dev/null | FileCheck -match-full-lines -check-prefix Thumbv6m-elf %s
+// Thumbv6m-elf: #define __ELF__ 1
+
+// RUN: %clang_cc1 -x c++ -E -dM -ffreestanding -triple=thumbv6m-none-unknown-eabi < /dev/null | FileCheck -match-full-lines -check-prefix Thumbv6m-cxx %s
+// Thumbv6m-cxx: #define _GNU_SOURCE 1
Index: clang/lib/Basic/Targets/ARM.cpp
===
--- clang/lib/Basic/Targets/ARM.cpp
+++ clang/lib/Basic/Targets/ARM.cpp
@@ -705,8 +705,11 @@
   // For bare-metal none-eabi.
   if (getTriple().getOS() == llvm::Triple::UnknownOS &&
   (getTriple().getEnvironment() == llvm::Triple::EABI ||
-   getTriple().getEnvironment() == llvm::Triple::EABIHF))
+   getTriple().getEnvironment() == llvm::Triple::EABIHF)) {
 Builder.defineMacro("__ELF__");
+if (Opts.CPlusPlus)
+  Builder.defineMacro("_GNU_SOURCE");
+  }
 
   // Target properties.
   Builder.defineMacro("__REGISTER_PREFIX__", "");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 2497d5a - Define _GNU_SOURCE for arm baremetal in C++ mode.

[Manoj Gupta via cfe-commits]

Author: Manoj Gupta
Date: 2022-11-03T13:58:47-07:00
New Revision: 2497d5aa7716a664c4f73df1980b026c906c7522

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

LOG: Define _GNU_SOURCE for arm baremetal in C++ mode.

This matches other C++ drivers e.g. Linux that define
_GNU_SOURCE. This lets clang compiler more code by default
without explicitly passing _GNU_SOURCE on command line.

Reviewed By: MaskRay

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

Added: 


Modified: 
clang/lib/Basic/Targets/ARM.cpp
clang/test/Preprocessor/init-arm.c

Removed: 




diff  --git a/clang/lib/Basic/Targets/ARM.cpp b/clang/lib/Basic/Targets/ARM.cpp
index f2db186aac4cb..c38849058e13d 100644
--- a/clang/lib/Basic/Targets/ARM.cpp
+++ b/clang/lib/Basic/Targets/ARM.cpp
@@ -705,8 +705,11 @@ void ARMTargetInfo::getTargetDefines(const LangOptions 
,
   // For bare-metal none-eabi.
   if (getTriple().getOS() == llvm::Triple::UnknownOS &&
   (getTriple().getEnvironment() == llvm::Triple::EABI ||
-   getTriple().getEnvironment() == llvm::Triple::EABIHF))
+   getTriple().getEnvironment() == llvm::Triple::EABIHF)) {
 Builder.defineMacro("__ELF__");
+if (Opts.CPlusPlus)
+  Builder.defineMacro("_GNU_SOURCE");
+  }
 
   // Target properties.
   Builder.defineMacro("__REGISTER_PREFIX__", "");

diff  --git a/clang/test/Preprocessor/init-arm.c 
b/clang/test/Preprocessor/init-arm.c
index e317ffa67393d..a55d0d63a79b3 100644
--- a/clang/test/Preprocessor/init-arm.c
+++ b/clang/test/Preprocessor/init-arm.c
@@ -1450,3 +1450,8 @@
 
 // THUMB-MINGW:#define __ARM_DWARF_EH__ 1
 
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=thumbv6m-none-unknown-eabi < 
/dev/null | FileCheck -match-full-lines -check-prefix Thumbv6m-elf %s
+// Thumbv6m-elf: #define __ELF__ 1
+
+// RUN: %clang_cc1 -x c++ -E -dM -ffreestanding 
-triple=thumbv6m-none-unknown-eabi < /dev/null | FileCheck -match-full-lines 
-check-prefix Thumbv6m-cxx %s
+// Thumbv6m-cxx: #define _GNU_SOURCE 1



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

[PATCH] D137024: [compiler-rt] Switch from llvm-config to find_package(LLVM)

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D137024#3905181 , @mgorny wrote:

>> These workarounds are fine for me, but I wonder if it would be useful with a 
>> more direct cmake option to tell it not look for these files at all.
>
> CMake has something like that built-in. I think it's 
> `-DCMAKE_DISABLE_FIND_PACKAGE_LLVM=ON`.

Oh, interesting, I wasn't aware of that one.

I thought more of the lines of telling compiler-rt to entirely skip the 
codepaths that try to look for these bits, but maybe it's not necessary in the 
end. Anyway, I'll follow this effort if there are more refactorings coming up 
in this area.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137024

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

[PATCH] D137338: Fix dupe word typos

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/CodeGen/CodeGenTBAA.cpp:341
 if (const CXXRecordDecl *CXXRD = dyn_cast(RD)) {
-  // Handle C++ base classes. Non-virtual bases can treated a a kind of
+  // Handle C++ base classes. Non-virtual bases can treated a kind of
   // field. Virtual bases are more complex and omitted, but avoid an

curdeius wrote:
> aaron.ballman wrote:
> > I think this should read:
> > ```
> >  // Handle C++ base classes. Non-virtual bases can treated as a kind of
> > ```
> A mistake: can *be* treated.
Oh gosh, good catch! 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137338

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

[PATCH] D137338: Fix dupe word typos

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/lib/CodeGen/CodeGenTBAA.cpp:341
 if (const CXXRecordDecl *CXXRD = dyn_cast(RD)) {
-  // Handle C++ base classes. Non-virtual bases can treated a a kind of
+  // Handle C++ base classes. Non-virtual bases can treated a kind of
   // field. Virtual bases are more complex and omitted, but avoid an

aaron.ballman wrote:
> I think this should read:
> ```
>  // Handle C++ base classes. Non-virtual bases can treated as a kind of
> ```
A mistake: can *be* treated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137338

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

[PATCH] D135987: [clangBasic] Refactor StaticAnalyzer to use `clang::SarifDocumentWriter`

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you for this refactoring, I really like the direction it's going! I've 
not spotted anything concerning, but if someone can double-check conformance to 
SARIF in terms of the changes to the tests, that would be appreciated.

It looks like precommit CI found a relevant failure that should be addressed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135987

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

[PATCH] D137340: [clang-tidy] Add modernize-use-anonymous-namespace check

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

I'm not entirely sure why this belongs in the modernize module given anonymous 
namespaces have been in c++ forever, maybe its more of a misc check? Also the 
modernize checks are meant to actually emit fixes(ignore the c arrays check :) 
), Right now, this only warn, it doesn't appear to act


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137340

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

[PATCH] D137350: [RISCV] Implement assembler support for XVentanaCondOps

[Philip Reames via Phabricator via cfe-commits]

reames updated this revision to Diff 473022.
reames added a comment.

Address review comments from @craig.topper


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

https://reviews.llvm.org/D137350

Files:
  clang/test/Preprocessor/riscv-target-features.c
  llvm/docs/RISCVUsage.rst
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVInstrFormats.td
  llvm/lib/Target/RISCV/RISCVInstrInfo.td
  llvm/lib/Target/RISCV/RISCVSubtarget.h
  llvm/test/CodeGen/RISCV/attributes.ll
  llvm/test/MC/RISCV/XVentanaCondOps-valid.s

Index: llvm/test/MC/RISCV/XVentanaCondOps-valid.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/XVentanaCondOps-valid.s
@@ -0,0 +1,22 @@
+# RUN: llvm-mc %s -triple=riscv64 -mattr=+xventanacondops -riscv-no-aliases -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-ASM,CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+xventanacondops < %s \
+# RUN: | llvm-objdump --mattr=+xventanacondops -M no-aliases -d -r - \
+# RUN: | FileCheck --check-prefix=CHECK-ASM-AND-OBJ %s
+
+# CHECK-ASM-AND-OBJ: vt.maskc zero, zero, zero
+# CHECK-ASM: encoding: [0x7b,0x60,0x00,0x00]
+vt.maskc x0, x0, x0
+
+# CHECK-ASM-AND-OBJ: vt.maskcn zero, zero, zero
+# CHECK-ASM: encoding: [0x7b,0x70,0x00,0x00]
+vt.maskcn x0, x0, x0
+
+# CHECK-ASM-AND-OBJ: vt.maskc ra, sp, gp
+# CHECK-ASM: encoding: [0xfb,0x60,0x31,0x00]
+vt.maskc x1, x2, x3
+
+# CHECK-ASM-AND-OBJ: vt.maskcn ra, sp, gp
+# CHECK-ASM: encoding: [0xfb,0x70,0x31,0x00]
+vt.maskcn x1, x2, x3
+
Index: llvm/test/CodeGen/RISCV/attributes.ll
===
--- llvm/test/CodeGen/RISCV/attributes.ll
+++ llvm/test/CodeGen/RISCV/attributes.ll
@@ -76,6 +76,7 @@
 ; RUN: llc -mtriple=riscv64 -mattr=+zicbop %s -o - | FileCheck --check-prefix=RV64ZICBOP %s
 ; RUN: llc -mtriple=riscv64 -mattr=+svnapot %s -o - | FileCheck --check-prefix=RV64SVNAPOT %s
 ; RUN: llc -mtriple=riscv64 -mattr=+svinval %s -o - | FileCheck --check-prefix=RV64SVINVAL %s
+; RUN: llc -mtriple=riscv64 -mattr=+xventanacondops %s -o - | FileCheck --check-prefix=RV64XVENTANACONDOPS %s
 ; RUN: llc -mtriple=riscv64 -mattr=+experimental-zawrs %s -o - | FileCheck --check-prefix=RV64ZAWRS %s
 ; RUN: llc -mtriple=riscv64 -mattr=+experimental-ztso %s -o - | FileCheck --check-prefix=RV64ZTSO %s
 
@@ -157,6 +158,7 @@
 ; RV64ZICBOP: .attribute 5, "rv64i2p0_zicbop1p0"
 ; RV64SVNAPOT: .attribute 5, "rv64i2p0_svnapot1p0"
 ; RV64SVINVAL: .attribute 5, "rv64i2p0_svinval1p0"
+; RV64XVENTANACONDOPS: .attribute 5, "rv64i2p0_xventanacondops1p0"
 ; RV64ZTSO: .attribute 5, "rv64i2p0_ztso0p1"
 
 define i32 @addi(i32 %a) {
Index: llvm/lib/Target/RISCV/RISCVSubtarget.h
===
--- llvm/lib/Target/RISCV/RISCVSubtarget.h
+++ llvm/lib/Target/RISCV/RISCVSubtarget.h
@@ -90,6 +90,7 @@
   bool HasStdExtZmmul = false;
   bool HasStdExtZawrs = false;
   bool HasStdExtZtso = false;
+  bool HasVendorXVentanaCondOps = false;
   bool HasRV32 = false;
   bool HasRV64 = false;
   bool IsRV32E = false;
@@ -189,6 +190,7 @@
   bool hasStdExtZawrs() const { return HasStdExtZawrs; }
   bool hasStdExtZmmul() const { return HasStdExtZmmul; }
   bool hasStdExtZtso() const { return HasStdExtZtso; }
+  bool hasVendorXVentanaCondOps() const { return HasVendorXVentanaCondOps; }
   bool is64Bit() const { return HasRV64; }
   bool isRV32E() const { return IsRV32E; }
   bool enableLinkerRelax() const { return EnableLinkerRelax; }
Index: llvm/lib/Target/RISCV/RISCVInstrInfo.td
===
--- llvm/lib/Target/RISCV/RISCVInstrInfo.td
+++ llvm/lib/Target/RISCV/RISCVInstrInfo.td
@@ -1780,3 +1780,24 @@
 include "RISCVInstrInfoV.td"
 include "RISCVInstrInfoZfh.td"
 include "RISCVInstrInfoZicbo.td"
+
+//===--===//
+// Vendor extensions
+//===--===//
+
+// -XVentanaCondOps
+let Predicates = [IsRV64, HasVendorXVentanaCondOps], hasSideEffects = 0,
+  mayLoad = 0, mayStore = 0, isCodeGenOnly = 0 in {
+
+class VTMaskedMove funct3, string opcodestr>
+: RVInstR<0b000, funct3, OPC_CUSTOM3, (outs GPR:$rd),
+  (ins GPR:$rs1, GPR:$rs2), opcodestr,
+  "$rd, $rs1, $rs2">{
+}
+
+def VT_MASKC : VTMaskedMove<0b110, "vt.maskc">,
+   Sched<[WriteIALU, ReadIALU, ReadIALU]>;
+
+def VT_MASKCN : VTMaskedMove<0b111, "vt.maskcn">,
+   Sched<[WriteIALU, ReadIALU, ReadIALU]>;
+}
Index: llvm/lib/Target/RISCV/RISCVInstrFormats.td
===
--- llvm/lib/Target/RISCV/RISCVInstrFormats.td
+++ llvm/lib/Target/RISCV/RISCVInstrFormats.td
@@ -145,6 +145,7 @@
 def OPC_JALR  : RISCVOpcode<"JALR",  0b1100111>;
 def OPC_JAL   : 

[PATCH] D137154: Adding nvvm_reflect clang builtin

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/include/clang/Basic/BuiltinsNVPTX.def:827
 
+BUILTIN(__nvvm_reflect, "icC*", "nc")
+

Do we actually need this patch at all.

`extern "C" int __nvvm_reflect(const char *);` appears to work just fine 
already:
https://godbolt.org/z/caGenxn1e





Comment at: clang/test/CodeGenCUDA/nvvm-reflect.cu:15
+// RUN:sm_50 -S -o /dev/null %s -mllvm -print-after-all 2>&1 \
+// RUN:| FileCheck %s --check-prefix=ARCH_REFLECT_1
+// RUN: %clang_cc1 -fcuda-is-device -triple nvptx64-nvidia-cuda -target-cpu \

Nit. I'd use the suffix matching the GPU variant we're targeting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137154

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

[PATCH] D137350: [RISCV] Implement assembler support for XVentanaCondOps

[Philip Reames via Phabricator via cfe-commits]

reames added a comment.

In D137350#3906126 , @craig.topper 
wrote:

> Do these need their own DecoderNameSpace?

What is a decoder namespace?  Some quick grepping and googling isn't very 
informative.




Comment at: llvm/lib/Target/RISCV/RISCVInstrFormats.td:148
 def OPC_SYSTEM: RISCVOpcode<"SYSTEM",0b1110011>;
+def OPC_CUSTOM3   : RISCVOpcode<"CUSTOM3",   0b011>;
 

craig.topper wrote:
> Since the string here is used for .insn parsing, I think it should be 
> CUSTOM_3. I'm not sure why we don't already have all 4 added.
https://reviews.llvm.org/D137355


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

https://reviews.llvm.org/D137350

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

[PATCH] D136811: -Wunsafe-buffer-usage: WIP: RFC: NFC: User documentation.

[Jan Korous via Phabricator via cfe-commits]

jkorous added a comment.

Thank you for the feedback Gábor!




Comment at: clang/docs/SafeBuffers.rst:36-37
+hardened mode where C++ classes such as ``std::vector`` and ``std::span``,
+together with their respective ``iterator`` classes, are protected
+at runtime against buffer overflows. This changes buffer overflows from
+extremely dangerous undefined behavior to predictable runtime crash.

xazax.hun wrote:
> Only buffer overflows, so comparing cases like:
> ```
> auto it1 = v1.begin();
> auto it2 = v2.begin();
> 
> if (it1 == it2) { ... }
> ```
> 
> are out of scope, right? 
> 
> The main reason I'm asking, because I am not sure how safe iterators are 
> implemented and whether the same implementation would give opportunities for 
> other safety checks without significant additional cost. 
Yes, this is out of scope for our diagnostics and code transformations. 
@ldionne Do you want to cover the safe iterator design part?



Comment at: clang/docs/SafeBuffers.rst:40-41
+  - Finally, in order to avoid bugs in newly converted code, the
+Clang static analyzer provides a checker to find misconstructed
+``std::span`` objects.
+

xazax.hun wrote:
> Isn't this overly specific? What about `string_view`s? 
This is based on our current plans for near future.
While we are positive we will use `std::span` in the FixIts it is very unlikely 
we'd use `string_view`. That's why having a checker for `std::span` is more 
important for us now.



Comment at: clang/docs/SafeBuffers.rst:287-288
+
+The automatic fixit associated with the warning would transform this array
+into an ``std::array``:
+

xazax.hun wrote:
> I anticipate that doing these fixits for multi-dimensional arrays will be 
> confusing. Is that in scope?
Do you mean that the dimensions get reversed if we replace `int 
arr_3d[3][4][5]` with `array, 4>,  3> arr_3d_r`?



Comment at: clang/docs/SafeBuffers.rst:324-326
+and binary compatibility. In order to avoid that, the fix will provide
+a compatibility overload to preserve the old functionality. The updated code
+produced by the fixit will look like this:

xazax.hun wrote:
> While this might be desirable for some projects, I can imagine other projects 
> want to avoid generating the overload (just generate the fixes in one pass, 
> and apply all of them in a second to avoid a "fixed" header render another TU 
> invalid). But that would require fixits for the call sites. Do you plan to 
> support those batch migration scenarios or is that out of scope?
We currently don't have a machinery that would allow us to analyze the whole 
project at once.

Two passes are generally not enough - that number is something like number of 
unique functions in the deepest call-stack.

Part of our fixits is attributing functions as unsafe and application of such 
fixits can create new warnings in callers.

Let's take an example:

```
foo(int* ptr) {
  ptr[5] = 5;
}

bar(int* ptr) {
  foo(ptr);
}

baz(int* ptr) {
  bar(ptr);
}
```

Initially it is not obvious that `bar` or `baz` might get transformed.

In the first iteration a fixit for `foo` is emitted and when applied we get:
```
foo(span ptr) {
  ptr[5] = 5;
}

[[clang::unsafe_buffer_usage]] foo(int* ptr);

bar(int* ptr) {
  foo(ptr);
}

baz(int* ptr) {
  bar(ptr);
}
```

Only now we learn that `bar` (which can live in a diferent TU) should get 
transformed as well because it calls to unsafe function `foo`. But our analysis 
still won't see that `baz` will eventually have to be transformed too and it'll 
take another iteration.



Comment at: clang/docs/SafeBuffers.rst:352
+
+  - The compatibility overload contains a ``__placeholder__`` which needs
+to be populated manually. In this case ``size`` is a good candidate.

aaron.ballman wrote:
> xazax.hun wrote:
> > Just a small concern. While users are not allowed to use identifiers like 
> > this, there is always a chance someone has a global variable of this name 
> > and the code ends up compiling. So, if we want to be absolutely safe, we 
> > should either have logic to pick a unique name, or not generate fixits in 
> > that case. Although, this is probably rare enough that we could just 
> > hand-wave.
> Agreed; we should not have a fixit that suggests use of a reserved identifier.
Our thinking is to actually use syntactically incorrect placeholder to make 
sure that users can't accidentally miss it.
This is related to:
https://reviews.llvm.org/D136811#inline-1324542



Comment at: clang/docs/SafeBuffers.rst:354-357
+  - Despite accepting a ``std::span`` which carries size information,
+the fixed function still accepts ``size`` separately. It can be removed
+manually, or it can be preserved, if ``size`` and ``buf.size()``
+actually need to be different in your case.

xazax.hun wrote:
> I see 

[PATCH] D136919: [X86][RFC] Change mangle name of __bf16 from u6__bf16 to DF16b

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D136919#3906133 , @rjmccall wrote:

> We talked about this on the Itanium list, and as currently specified, it is 
> absolutely not correct for `__bf16` to have the same mangling as 
> `std::bfloat16_t`, because `__bf16` does not have the correct semantics for 
> `std::bfloat16_t` and must be a distinct type.  If GCC changed `__bf16` to 
> use the new mangling without also updating the semantics, it is a bug.
>
> That discussion was here: https://github.com/itanium-cxx-abi/cxx-abi/pull/147
>
> If we want to implement `std::bfloat16_t` in Clang, we need to make it a 
> normal arithmetic type, and in practice it needs to guarantee 
> excess-precision arithmetic, as I discussed on that thread.  Coincidentally, 
> we did recently implement excess-precision arithmetic in Clang for `_Float16`.

Jakub Jelinek has clarified that GCC did indeed change the semantics of 
`__bf16` on i386 and x86_64 to be a proper extended floating point type.

We could change the mangling to match GCC, but I think it would be 
inappropriate to do that without also matching the semantics change.  Since the 
mangling change is trivial to land, I think the semantics change should happen 
first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136919

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

[PATCH] D136282: [clang] [CMake] Link libclangBasic against libatomic when necessary.

[Michał Górny via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG20132d8eaa68: Link libclangBasic against libatomic when 
necessary. (authored by thesamesam, committed by mgorny).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136282

Files:
  clang/CMakeLists.txt
  clang/lib/Basic/CMakeLists.txt


Index: clang/lib/Basic/CMakeLists.txt
===
--- clang/lib/Basic/CMakeLists.txt
+++ clang/lib/Basic/CMakeLists.txt
@@ -111,3 +111,7 @@
   omp_gen
   )
 
+target_link_libraries(clangBasic
+  PRIVATE
+  ${LLVM_ATOMIC_LIB}
+)
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -63,6 +63,7 @@
   include(TableGen)
   include(HandleLLVMOptions)
   include(VersionFromVCS)
+  include(CheckAtomic)
   include(GetErrcMessages)
   include(LLVMDistributionSupport)
 


Index: clang/lib/Basic/CMakeLists.txt
===
--- clang/lib/Basic/CMakeLists.txt
+++ clang/lib/Basic/CMakeLists.txt
@@ -111,3 +111,7 @@
   omp_gen
   )
 
+target_link_libraries(clangBasic
+  PRIVATE
+  ${LLVM_ATOMIC_LIB}
+)
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -63,6 +63,7 @@
   include(TableGen)
   include(HandleLLVMOptions)
   include(VersionFromVCS)
+  include(CheckAtomic)
   include(GetErrcMessages)
   include(LLVMDistributionSupport)
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 20132d8 - Link libclangBasic against libatomic when necessary.

[Michał Górny via cfe-commits]

Author: Sam James
Date: 2022-11-03T21:07:44+01:00
New Revision: 20132d8eaa68a6c53e152718beda1dc0f4c9ff6c

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

LOG: Link libclangBasic against libatomic when necessary.

This is necessary at least on PPC32.

Depends on D136280.

Bug: https://bugs.gentoo.org/874024
Thanks-to: Arfrever Frehtes Taifersar Arahesis 
Tested-by: erhar...@mailbox.org 

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

Added: 


Modified: 
clang/CMakeLists.txt
clang/lib/Basic/CMakeLists.txt

Removed: 




diff  --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt
index 2ca81e506338c..236e6fbaca280 100644
--- a/clang/CMakeLists.txt
+++ b/clang/CMakeLists.txt
@@ -63,6 +63,7 @@ if(CLANG_BUILT_STANDALONE)
   include(TableGen)
   include(HandleLLVMOptions)
   include(VersionFromVCS)
+  include(CheckAtomic)
   include(GetErrcMessages)
   include(LLVMDistributionSupport)
 

diff  --git a/clang/lib/Basic/CMakeLists.txt b/clang/lib/Basic/CMakeLists.txt
index 5d197f59ac4f7..f0f3839a7e2c3 100644
--- a/clang/lib/Basic/CMakeLists.txt
+++ b/clang/lib/Basic/CMakeLists.txt
@@ -111,3 +111,7 @@ add_clang_library(clangBasic
   omp_gen
   )
 
+target_link_libraries(clangBasic
+  PRIVATE
+  ${LLVM_ATOMIC_LIB}
+)



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

[PATCH] D136796: [llvm-objdump][Offload] Use common offload extraction method

[Artem Belevich via Phabricator via cfe-commits]

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

LGTM, modulo separating clang-related  change into a separate patch.




Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:1262-1265
+if (identify_magic((*BufferOrErr)->getBuffer()) ==
+file_magic::elf_shared_object)
+  continue;
+

jhenderson wrote:
> This change seems not really part of the patch, since it's touching `clang` 
> but the patch is for `llvm-objdump`.
+1 to landing this separately, possibly with a relevant test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136796

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

[PATCH] D135721: [HLSL] Added HLSL this as a reference

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:179
+AST, SourceLocation(),
+Constructor->getThisType().getTypePtr()->getPointeeType(), true);
+This->setValueKind(ExprValueKind::VK_LValue);

aaron.ballman wrote:
> gracejennings wrote:
> > aaron.ballman wrote:
> > > python3kgae wrote:
> > > > gracejennings wrote:
> > > > > python3kgae wrote:
> > > > > > gracejennings wrote:
> > > > > > > python3kgae wrote:
> > > > > > > > gracejennings wrote:
> > > > > > > > > python3kgae wrote:
> > > > > > > > > > Should this be a reference type?
> > > > > > > > > Could you expand on the question? I'm not sure I understand 
> > > > > > > > > what you're asking. The two changes in this file were made to 
> > > > > > > > > update the previous RWBuffer implementation
> > > > > > > > The current code will create CXXThisExpr with the pointeeType.
> > > > > > > > I thought it should be a reference type of the pointeeType.
> > > > > > > > 
> > > > > > > > Like in the test,
> > > > > > > > CXXThisExpr 0x{{[0-9A-Fa-f]+}} <> 
> > > > > > > > 'RWBuffer *' implicit this
> > > > > > > > 
> > > > > > > > The type is RWBuffer * before,
> > > > > > > > I expected this patch will change it to
> > > > > > > > RWBuffer &.
> > > > > > > The change that makes it more reference like than c++ from:
> > > > > > > 
> > > > > > > `-MemberExpr 0x{{[0-9A-Fa-f]+}}  'int' lvalue 
> > > > > > > ->First 0x{{[0-9A-Fa-f]+}}`
> > > > > > > `-CXXThisExpr 0x{{[0-9A-Fa-f]+}}  'Pair *' this`
> > > > > > > 
> > > > > > > to hlsl with this change
> > > > > > > 
> > > > > > > `-MemberExpr 0x{{[0-9A-Fa-f]+}}  'int' lvalue 
> > > > > > > .First 0x{{[0-9A-Fa-f]+}}`
> > > > > > > `-CXXThisExpr 0x{{[0-9A-Fa-f]+}}  'Pair' lvalue this`
> > > > > > I guess we have to change clang codeGen for this anyway.
> > > > > > 
> > > > > > Not sure which has less impact for codeGen side,  lvalue like what 
> > > > > > is in the current patch or make it a lvalue reference? My feeling 
> > > > > > is lvalue reference might be eaiser.
> > > > > > 
> > > > > > Did you test what needs to change for clang codeGen on top of the 
> > > > > > current patch?
> > > > > > 
> > > > > With just the lvalue change in the current patch there should be no 
> > > > > additional changes needed in clang CodeGen on top of the current patch
> > > > Since we already get the codeGen working with this patch,
> > > > it would be nice to have a codeGen test.
> > > I think it's an interesting question to consider, but I have some 
> > > concerns. Consider code like this:
> > > ```
> > > struct S {
> > >   int Val = 0;
> > >   void foo() {
> > > Val = 10;
> > > S Another;
> > > this = Another; // If this is a non-const reference, we can assign 
> > > into it...
> > > print(); // ... so do we print 0 or 10?
> > > // When this function ends, is `this` destroyed because `Another` 
> > > goes out of scope?
> > >   }
> > >   void print() {
> > > std::cout << Val;
> > >   }
> > > };
> > > ```
> > > I think we need to prevent code like that from working. But it's not just 
> > > direct assignments that are a concern. Consider:
> > > ```
> > > struct S;
> > > 
> > > void func(S , S ) {
> > >   Out = In;
> > > }
> > > 
> > > struct S {
> > >   int Val = 0;
> > >   void foo() {
> > > Val = 10;
> > > S Another;
> > > func(this, Another); // Same problem here!
> > > print();
> > >   }
> > >   void print() {
> > > std::cout << Val;
> > >   }
> > > };
> > > ```
> > > Another situation that I'm not certain of is whether HLSL supports 
> > > tail-allocation where the code is doing something like `this + 1` to get 
> > > to the start of the trailing objects.
> > For the first example we would expect this to work for HLSL because `this` 
> > is reference like (with modifications to make it supported by HLSL). We 
> > would expect `Val` to be `0`, printing `0`, and `Another` to be destroyed, 
> > but not `this`. I went ahead and added similar CodeGen test coverage.
> > 
> > For the second example, there is not reference support in HLSL. Changing to 
> > a similar example with copy-in and copy-out to make it HLSL supported would 
> > take care of that case, but copy-in/out is not supported upstream yet. 
> > For the first example we would expect this to work for HLSL because this is 
> > reference like (with modifications to make it supported by HLSL). We would 
> > expect Val to be 0, printing 0, and Another to be destroyed, but not this. 
> > I went ahead and added similar CodeGen test coverage.
> 
> I was surprised about the dangers of that design, so I spoke with @beanz over 
> IRC about it and he was saying that the goal for HLSL was for that code to 
> call the copy assignment operator and not do a reference replacement, so it'd 
> behave more like `*this` in C++, as in: https://godbolt.org/z/qrEav6sjq. That 
> design makes a lot more sense to me, but I'm also not at all an expert on 
> HLSL, 

[PATCH] D137338: Fix dupe word typos

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

The clang parts generally LG, but I spotted a few things. Thank you for this 
cleanup effort!




Comment at: clang/include/clang/ASTMatchers/ASTMatchersInternal.h:467
 
-  /// Return a matcher that that points to the same implementation, but sets 
the
+  /// Return a matcher that points to the same implementation, but sets the
   ///   traversal kind.

This is a good change, but you also need to run 
`clang/docs/tools/dump_ast_matchers.py` to regenerate the public-facing 
documentation that is generated from this file.



Comment at: clang/lib/CodeGen/CodeGenTBAA.cpp:341
 if (const CXXRecordDecl *CXXRD = dyn_cast(RD)) {
-  // Handle C++ base classes. Non-virtual bases can treated a a kind of
+  // Handle C++ base classes. Non-virtual bases can treated a kind of
   // field. Virtual bases are more complex and omitted, but avoid an

I think this should read:
```
 // Handle C++ base classes. Non-virtual bases can treated as a kind of
```



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:47
   // complete architecture list, nor a reasonable subset. The problem is that
-  // historically the driver driver accepts this and also ties its -march=
   // handling to the architecture name, so we need to be careful before 
removing

This is duplicated just often enough and in just enough contexts where I think 
"driver driver" means "the driver that drives another driver", but it'd be nice 
if someone else could confirm or deny that.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:87
 
-  // If using a driver driver, force the arch.
   if (getToolChain().getTriple().isOSDarwin()) {

Same for this one as above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137338

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

[PATCH] D136919: [X86][RFC] Change mangle name of __bf16 from u6__bf16 to DF16b

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

We talked about this on the Itanium list, and as currently specified, it is 
absolutely not correct for `__bf16` to have the same mangling as 
`std::bfloat16_t`, because these types do not have the same semantics.  If GCC 
did that, it is a bug.

That discussion was here: https://github.com/itanium-cxx-abi/cxx-abi/pull/147


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136919

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

[PATCH] D136554: Implement CWG2631

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin marked an inline comment as done and an inline comment as not done.
cor3ntin added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:5918
+if (const FunctionDecl *FD = E->getDirectCallee())
+  HasImmediateCalls |= FD->isConsteval();
+return RecursiveASTVisitor::VisitStmt(E);

cor3ntin wrote:
> @shafk 
Oups, nvm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136554

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

[PATCH] D136554: Implement CWG2631

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin marked 8 inline comments as done.
cor3ntin added inline comments.



Comment at: clang/include/clang/Sema/Sema.h:9602-9604
+return Ctx.Context ==
+   ExpressionEvaluationContext::PotentiallyEvaluatedIfUsed ||
+   Ctx.IsCheckingDefaultArgumentOrInitializer;

shafik wrote:
> aaron.ballman wrote:
> > Hmm, it'd be nice to not name this with the same identifier as the `bool` 
> > member on line 1333, that surprised me a little bit when I ran into it 
> > below.
> Why are we logically ORing an enumerator w/ a `bool`?
I added parentheses here to make that clearer!



Comment at: clang/lib/Sema/SemaExpr.cpp:5918
+if (const FunctionDecl *FD = E->getDirectCallee())
+  HasImmediateCalls |= FD->isConsteval();
+return RecursiveASTVisitor::VisitStmt(E);

@shafk 



Comment at: clang/test/SemaCXX/cxx2a-consteval-default-params.cpp:10
+{
+return undefined();  // expected-error {{not a constant expression}} \
+ // expected-note  {{undefined function 
'undefined'}}

cor3ntin wrote:
> shafik wrote:
> > I don't think we expect a diagnostic here since `check_lambdas_unused` is 
> > never called.
> We do because the body of a lambda is not considered a subexpression. I 
> confirmed that with core.
> 
> See https://lists.isocpp.org/core/2022/10/13364.php
I added a comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136554

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

[PATCH] D137350: [RISCV] Implement assembler support for XVentanaCondOps

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

Do these need their own DecoderNameSpace?


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

https://reviews.llvm.org/D137350

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

[PATCH] D137338: Fix dupe word typos

[Jez Ng via Phabricator via cfe-commits]

int3 added a comment.

lld/MachO changes lgtm




Comment at: lld/MachO/Driver.cpp:1386
 // ld64 does something really weird. It attempts to realign the value to 
the
-// page size, but assumes the the page size is 4K. This doesn't work with
+// page size, but assumes the page size is 4K. This doesn't work with
 // most of Apple's ARM64 devices, which use a page size of 16K. This means

"assumes that the page size" sounds more natural and is probably what was 
intended



Comment at: lld/MachO/UnwindInfoSection.cpp:576-577
 // entries by using the regular format. This can happen when there
-// are many unique encodings, and we we saturated the local
+// are many unique encodings, and we saturated the local
 // encoding table early.
 if (i < cuIndices.size() &&

I think we can reflow this paragraph to max the line length


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137338

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

[PATCH] D136554: Implement CWG2631

[Corentin Jabot via Phabricator via cfe-commits]

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

Add parentheses to make check clearer


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136554

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/Parse/ParseCXXInlineMethods.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/CXX/class/class.local/p1-0x.cpp
  clang/test/CodeGenCXX/default-arguments-with-immediate.cpp
  clang/test/PCH/default-argument-with-immediate-calls.cpp
  clang/test/SemaCXX/cxx2a-consteval-default-params.cpp
  clang/test/SemaCXX/source_location.cpp

Index: clang/test/SemaCXX/source_location.cpp
===
--- clang/test/SemaCXX/source_location.cpp
+++ clang/test/SemaCXX/source_location.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions -verify %s
+// RUN: %clang_cc1 -std=c++2a -fcxx-exceptions -DUSE_CONSTEVAL -fexceptions -verify %s
 // expected-no-diagnostics
 
 #define assert(...) ((__VA_ARGS__) ? ((void)0) : throw 42)
@@ -8,15 +9,22 @@
 template 
 struct Printer;
 
+#ifdef USE_CONSTEVAL
+#define SOURCE_LOC_EVAL_KIND consteval
+#else
+#define SOURCE_LOC_EVAL_KIND constexpr
+#endif
+
 namespace std {
 class source_location {
   struct __impl;
 
 public:
-  static constexpr source_location current(const __impl *__p = __builtin_source_location()) noexcept {
-source_location __loc;
-__loc.__m_impl = __p;
-return __loc;
+  static SOURCE_LOC_EVAL_KIND source_location
+current(const __impl *__p = __builtin_source_location()) noexcept {
+  source_location __loc;
+  __loc.__m_impl = __p;
+  return __loc;
   }
   constexpr source_location() = default;
   constexpr source_location(source_location const &) = default;
@@ -593,3 +601,51 @@
   }
   static_assert(test());
 }
+
+namespace Lambda {
+#line 8000 "TestLambda.cpp"
+constexpr int nested_lambda(int l = []{
+  return SL::current().line();
+}()) {
+  return l;
+}
+static_assert(nested_lambda() == __LINE__ - 4);
+
+constexpr int lambda_param(int l = [](int l = SL::current().line()) {
+  return l;
+}()) {
+  return l;
+}
+static_assert(lambda_param() == __LINE__);
+
+
+}
+
+constexpr int compound_literal_fun(int a =
+  (int){ SL::current().line() }
+) { return a ;}
+static_assert(compound_literal_fun() == __LINE__);
+
+struct CompoundLiteral {
+  int a = (int){ SL::current().line() };
+};
+static_assert(CompoundLiteral{}.a == __LINE__);
+
+
+// FIXME
+// Init captures are subexpressions of the lambda expression
+// so according to the standard immediate invocations in init captures
+// should be evaluated at the call site.
+// However Clang does not yet implement this as it would introduce
+// a fair bit of complexity.
+// We intend to implement that functionality once we find real world
+// use cases that require it.
+constexpr int test_init_capture(int a =
+[b = SL::current().line()] { return b; }()) {
+  return a;
+}
+#ifdef USE_CONSTEVAL
+static_assert(test_init_capture() == __LINE__ - 4);
+#else
+static_assert(test_init_capture() == __LINE__ );
+#endif
Index: clang/test/SemaCXX/cxx2a-consteval-default-params.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2a-consteval-default-params.cpp
@@ -0,0 +1,68 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++2b %s
+
+consteval int undefined();  // expected-note 4 {{declared here}}
+
+void check_lambdas_unused(
+int a = []
+{
+// The body of a lambda is not a subexpression of the lambda
+// so this is immediately evaluated even if the parameter
+// is never used.
+return undefined();  // expected-error {{not a constant expression}} \
+ // expected-note  {{undefined function 'undefined'}}
+}(),
+int b = [](int no_error = undefined()) {
+return no_error;
+}(0),
+int c = [](int defaulted = undefined()) {
+return defaulted;
+}()
+) {}
+
+int check_lambdas_used(
+int b = [](int no_error = undefined()) {
+return no_error;
+}(0),
+int c = [](int defaulted = undefined()) { // expected-error {{not a constant expression}} \
+  // expected-note  {{declared here}} \
+  // expected-note  {{undefined function 'undefined'}}
+return defaulted;
+}(),  // expected-note {{in 

[PATCH] D137338: Fix dupe word typos

[Amir Ayupov via Phabricator via cfe-commits]

Amir accepted this revision.
Amir added a comment.

BOLT changes LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137338

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

[PATCH] D137350: [RISCV] Implement assembler support for XVentanaCondOps

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/test/Preprocessor/riscv-target-features.c:437
+
+// RUN: %clang -target riscv64 -march=rv64ixventanacondops -x c -E -dM %s \
+// RUN: -o - | FileCheck --check-prefix=CHECK-XVENTANACONDOPS-EXT %s

reames wrote:
> Naming wise, is xventanacondops what we want this called?  The toolchain docs 
> linked are a bit ambiguous on this point.  They seem to be saying that the 
> extension should maybe be xvtcondops.  But that's not what the spec uses, not 
> what we tend to use in discussion, and not what is being discussed for 
> binutils.
qemu seems to have also used xventanacondops.



Comment at: llvm/lib/Target/RISCV/RISCV.td:422
+: SubtargetFeature<"xventanacondops", "HasVendorXVentanaCondOps", "true",
+   "'XVentanaCondOps' (Ventana Conditional Move)">;
+def HasVendorXVentanaCondOps : 
Predicate<"Subtarget->hasVendorXVentanaCondOps()">,

It's not quite conditional move. It's conditional move or zero right? Might be 
better just says "Ops" or "Operations" instead of "Move".



Comment at: llvm/lib/Target/RISCV/RISCV.td:426
+"'XVentanaCondOps' (Ventana Conditional 
Move)">;
+//===--===//
+// LLVM specific features and extensions

Blank line above this.



Comment at: llvm/lib/Target/RISCV/RISCVInstrFormats.td:148
 def OPC_SYSTEM: RISCVOpcode<"SYSTEM",0b1110011>;
+def OPC_CUSTOM3   : RISCVOpcode<"CUSTOM3",   0b011>;
 

Since the string here is used for .insn parsing, I think it should be CUSTOM_3. 
I'm not sure why we don't already have all 4 added.


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

https://reviews.llvm.org/D137350

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

[PATCH] D136712: Define _GNU_SOURCE for arm baremetal in C++ mode.

[Manoj Gupta via Phabricator via cfe-commits]

manojgupta updated this revision to Diff 473004.
manojgupta added a comment.

Updated test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136712

Files:
  clang/lib/Basic/Targets/ARM.cpp
  clang/test/Preprocessor/init-arm.c


Index: clang/test/Preprocessor/init-arm.c
===
--- clang/test/Preprocessor/init-arm.c
+++ clang/test/Preprocessor/init-arm.c
@@ -1450,3 +1450,8 @@
 
 // THUMB-MINGW:#define __ARM_DWARF_EH__ 1
 
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=thumbv6m-none-unknown-eabi < 
/dev/null | FileCheck -match-full-lines -check-prefix Thumbv6m-elf %s
+// Thumbv6m-elf: #define __ELF__ 1
+
+// RUN: %clang_cc1 -x c++ -E -dM -ffreestanding 
-triple=thumbv6m-none-unknown-eabi < /dev/null | FileCheck -match-full-lines 
-check-prefix Thumbv6m-cxx %s
+// Thumbv6m-cxx: #define _GNU_SOURCE 1
Index: clang/lib/Basic/Targets/ARM.cpp
===
--- clang/lib/Basic/Targets/ARM.cpp
+++ clang/lib/Basic/Targets/ARM.cpp
@@ -705,8 +705,11 @@
   // For bare-metal none-eabi.
   if (getTriple().getOS() == llvm::Triple::UnknownOS &&
   (getTriple().getEnvironment() == llvm::Triple::EABI ||
-   getTriple().getEnvironment() == llvm::Triple::EABIHF))
+   getTriple().getEnvironment() == llvm::Triple::EABIHF)) {
 Builder.defineMacro("__ELF__");
+if (Opts.CPlusPlus)
+  Builder.defineMacro("_GNU_SOURCE");
+  }
 
   // Target properties.
   Builder.defineMacro("__REGISTER_PREFIX__", "");


Index: clang/test/Preprocessor/init-arm.c
===
--- clang/test/Preprocessor/init-arm.c
+++ clang/test/Preprocessor/init-arm.c
@@ -1450,3 +1450,8 @@
 
 // THUMB-MINGW:#define __ARM_DWARF_EH__ 1
 
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=thumbv6m-none-unknown-eabi < /dev/null | FileCheck -match-full-lines -check-prefix Thumbv6m-elf %s
+// Thumbv6m-elf: #define __ELF__ 1
+
+// RUN: %clang_cc1 -x c++ -E -dM -ffreestanding -triple=thumbv6m-none-unknown-eabi < /dev/null | FileCheck -match-full-lines -check-prefix Thumbv6m-cxx %s
+// Thumbv6m-cxx: #define _GNU_SOURCE 1
Index: clang/lib/Basic/Targets/ARM.cpp
===
--- clang/lib/Basic/Targets/ARM.cpp
+++ clang/lib/Basic/Targets/ARM.cpp
@@ -705,8 +705,11 @@
   // For bare-metal none-eabi.
   if (getTriple().getOS() == llvm::Triple::UnknownOS &&
   (getTriple().getEnvironment() == llvm::Triple::EABI ||
-   getTriple().getEnvironment() == llvm::Triple::EABIHF))
+   getTriple().getEnvironment() == llvm::Triple::EABIHF)) {
 Builder.defineMacro("__ELF__");
+if (Opts.CPlusPlus)
+  Builder.defineMacro("_GNU_SOURCE");
+  }
 
   // Target properties.
   Builder.defineMacro("__REGISTER_PREFIX__", "");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136919: [X86][RFC] Change mangle name of __bf16 from u6__bf16 to DF16b

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: clang-vendors, rjmccall.
aaron.ballman added a subscriber: rjmccall.
aaron.ballman added a comment.

Adding clang-vendors because of the potential ABI break concerns and @rjmccall 
for the Itanium ABI questions specifically.

In D136919#3904925 , @RKSimon wrote:

> What are the rules on this? Do we just handle this as an ABI breaking change 
> and document it in the release notes - or do we need to provide any 
> auto-upgrade path (with a warning?)?

I don't know that we have hard and fast rules for this, we usually handle it on 
a case by case basis. If users are relying on the existing mangling, we may 
want to provide an ABI compat check so users can get the old mangling behavior. 
We definitely need to add a release note calling this change out, and we should 
probably also announce it on discourse once the changes land.




Comment at: clang/lib/Basic/Targets/X86.h:413
 
-  const char *getBFloat16Mangling() const override { return "u6__bf16"; };
+  const char *getBFloat16Mangling() const override { return "DF16b"; };
 };

This does not appear to match the Itanium ABI 
(https://itanium-cxx-abi.github.io/cxx-abi/abi.html):
```
 ::= DF  _ # ISO/IEC TS 18661 binary floating point type _FloatN (N 
bits)
```
it's missing the underscore after the number of bits. Is there a proposal in 
front of the Itanium ABI group to add this form with `b` instead of an 
underscore?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136919

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