[PATCH] D50979: Eliminate instances of `EmitScalarExpr(E->getArg(n))` in EmitX86BuiltinExpr().

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

> EmitAArch64BuiltinExpr() also emits args into Ops before the big switch (with 
> some more subtlety around the last arg that I don't understand), but then 
> almost every switch case does EmitScalarExpr(E->getArg(n)).

Took me a while to remember, but I can at least explain this bit now I think. 
The key is that there are two kinds of NEON intrinsics. Overloaded ones have a 
constant last argument that describes the real type, and non-overloaded ones 
use that last argument as a normal parameter.

The first massive switch you see handles the ones in the second case, so it 
always CodeGens the last parameter, but if you scroll all the way down to line 
7369 there's another switch where only the pregenerated Ops are used, and this 
last arg is visible as `Ty` and/or `VTy`.

It may or may not be the best way to handle that situation, of course.


https://reviews.llvm.org/D50979



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

[PATCH] D51683: Fix arm_neon.h and arm_fp16.h generation for compiling with std=c89

[Tim Northover via Phabricator via cfe-commits]

t.p.northover accepted this revision.
t.p.northover added a comment.
This revision is now accepted and ready to land.

Looks good to me.


Repository:
  rC Clang

https://reviews.llvm.org/D51683



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

[PATCH] D33568: Fix crash when evaluating constant expressions involving nullptr

[Tim Northover via Phabricator via cfe-commits]

t.p.northover created this revision.
Herald added a subscriber: mcrosier.

For the simple casts in the test, we were crashing because the 
ZeroInitialization function created an LValue with an unexpected 
SubobjectDesignator: it was valid but didn't actually refer to a valid 
MostDerivedType.

Since PointerExprEvaluator actually creates an LValue based notionally on what 
you'd get if you dereferenced the pointer (I think), the MostDerivedType should 
be set by stripping the pointer from the type as in this patch.

The theoretically simpler solution of providing an invalid SubobjectDesignator 
(as happens for an int to pointer cast) is incorrect because nullptr has more 
possible uses in constexprs than other casts of integers.

Does my reasoning look sound? I've been all over that file before I finally 
thought I knew what was going on and I'm still not entirely confident.


https://reviews.llvm.org/D33568

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/null-cast.cpp


Index: clang/test/SemaCXX/null-cast.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/null-cast.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct A {};
+struct B : virtual A {};
+
+void foo() {
+  (void)static_cast(*(B *)0); // expected-warning {{binding dereferenced 
null pointer to reference has undefined behavior}}
+}
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -5496,6 +5496,8 @@
   bool ZeroInitialization(const Expr *E) {
 auto Offset = Info.Ctx.getTargetNullPointerValue(E->getType());
 Result.set((Expr*)nullptr, 0, false, true, Offset);
+Result.getLValueDesignator() =
+SubobjectDesignator(E->getType()->getPointeeType());
 return true;
   }
 


Index: clang/test/SemaCXX/null-cast.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/null-cast.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct A {};
+struct B : virtual A {};
+
+void foo() {
+  (void)static_cast(*(B *)0); // expected-warning {{binding dereferenced null pointer to reference has undefined behavior}}
+}
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -5496,6 +5496,8 @@
   bool ZeroInitialization(const Expr *E) {
 auto Offset = Info.Ctx.getTargetNullPointerValue(E->getType());
 Result.set((Expr*)nullptr, 0, false, true, Offset);
+Result.getLValueDesignator() =
+SubobjectDesignator(E->getType()->getPointeeType());
 return true;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33568: Fix crash when evaluating constant expressions involving nullptr

[Tim Northover via Phabricator via cfe-commits]

t.p.northover updated this revision to Diff 100321.
t.p.northover added a comment.

Sounds very reasonable to me. I've uploaded a new diff.


https://reviews.llvm.org/D33568

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/null-cast.cpp


Index: clang/test/SemaCXX/null-cast.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/null-cast.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct A {};
+struct B : virtual A {};
+
+void foo() {
+  (void)static_cast(*(B *)0); // expected-warning {{binding dereferenced 
null pointer to reference has undefined behavior}}
+}
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -1230,8 +1230,7 @@
   IsNullPtr = V.isNullPointer();
 }
 
-void set(APValue::LValueBase B, unsigned I = 0, bool BInvalid = false,
- bool IsNullPtr_ = false, uint64_t Offset_ = 0) {
+void set(APValue::LValueBase B, unsigned I = 0, bool BInvalid = false) {
 #ifndef NDEBUG
   // We only allow a few types of invalid bases. Enforce that here.
   if (BInvalid) {
@@ -1242,11 +1241,20 @@
 #endif
 
   Base = B;
-  Offset = CharUnits::fromQuantity(Offset_);
+  Offset = CharUnits::fromQuantity(0);
   InvalidBase = BInvalid;
   CallIndex = I;
   Designator = SubobjectDesignator(getType(B));
-  IsNullPtr = IsNullPtr_;
+  IsNullPtr = false;
+}
+
+void setNull(QualType PointerTy, uint64_t TargetVal) {
+  Base = (Expr *)nullptr;
+  Offset = CharUnits::fromQuantity(TargetVal);
+  InvalidBase = false;
+  CallIndex = 0;
+  Designator = SubobjectDesignator(PointerTy->getPointeeType());
+  IsNullPtr = true;
 }
 
 void setInvalid(APValue::LValueBase B, unsigned I = 0) {
@@ -5494,8 +5502,8 @@
 return true;
   }
   bool ZeroInitialization(const Expr *E) {
-auto Offset = Info.Ctx.getTargetNullPointerValue(E->getType());
-Result.set((Expr*)nullptr, 0, false, true, Offset);
+auto TargetVal = Info.Ctx.getTargetNullPointerValue(E->getType());
+Result.setNull(E->getType(), TargetVal);
 return true;
   }
 


Index: clang/test/SemaCXX/null-cast.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/null-cast.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct A {};
+struct B : virtual A {};
+
+void foo() {
+  (void)static_cast(*(B *)0); // expected-warning {{binding dereferenced null pointer to reference has undefined behavior}}
+}
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -1230,8 +1230,7 @@
   IsNullPtr = V.isNullPointer();
 }
 
-void set(APValue::LValueBase B, unsigned I = 0, bool BInvalid = false,
- bool IsNullPtr_ = false, uint64_t Offset_ = 0) {
+void set(APValue::LValueBase B, unsigned I = 0, bool BInvalid = false) {
 #ifndef NDEBUG
   // We only allow a few types of invalid bases. Enforce that here.
   if (BInvalid) {
@@ -1242,11 +1241,20 @@
 #endif
 
   Base = B;
-  Offset = CharUnits::fromQuantity(Offset_);
+  Offset = CharUnits::fromQuantity(0);
   InvalidBase = BInvalid;
   CallIndex = I;
   Designator = SubobjectDesignator(getType(B));
-  IsNullPtr = IsNullPtr_;
+  IsNullPtr = false;
+}
+
+void setNull(QualType PointerTy, uint64_t TargetVal) {
+  Base = (Expr *)nullptr;
+  Offset = CharUnits::fromQuantity(TargetVal);
+  InvalidBase = false;
+  CallIndex = 0;
+  Designator = SubobjectDesignator(PointerTy->getPointeeType());
+  IsNullPtr = true;
 }
 
 void setInvalid(APValue::LValueBase B, unsigned I = 0) {
@@ -5494,8 +5502,8 @@
 return true;
   }
   bool ZeroInitialization(const Expr *E) {
-auto Offset = Info.Ctx.getTargetNullPointerValue(E->getType());
-Result.set((Expr*)nullptr, 0, false, true, Offset);
+auto TargetVal = Info.Ctx.getTargetNullPointerValue(E->getType());
+Result.setNull(E->getType(), TargetVal);
 return true;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33568: Fix crash when evaluating constant expressions involving nullptr

[Tim Northover via Phabricator via cfe-commits]

t.p.northover closed this revision.
t.p.northover added a comment.

Thanks Richard. I've committed this as r303957.


https://reviews.llvm.org/D33568



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

[PATCH] D33610: Stop asserting on constexpr explicit MS constructor calls.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover created this revision.
Herald added a subscriber: mcrosier.

When trying to switch to C++14 by default, some extra expressions become 
constexpr and this is one case we can't handle.

Technically I believe weakening the assert would be enough to fix the problem, 
but defaulted default constructors are extremely close to being allowed through 
which would lead to incorrect code (all of isDefaulted, isTrivial and hasFields 
contribute to the exclusion, as well as the fact that unions don't have default 
constructors). So I decided to explicitly check that we are dealing with a 
copy/move before executing that block.


https://reviews.llvm.org/D33610

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/CodeGenCXX/constructor-direct-call.cpp


Index: clang/test/CodeGenCXX/constructor-direct-call.cpp
===
--- clang/test/CodeGenCXX/constructor-direct-call.cpp
+++ clang/test/CodeGenCXX/constructor-direct-call.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple i686-pc-mingw32 -fms-extensions -Wmicrosoft %s 
-emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple i686-pc-mingw32 -fms-extensions -Wmicrosoft %s 
-emit-llvm -o - -std=gnu++11| FileCheck %s
 
 class Test1 {
 public:
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -4184,18 +4184,27 @@
 
   CallStackFrame Frame(Info, CallLoc, Callee, This, ArgValues.data());
 
+  const CXXMethodDecl *MD = dyn_cast(Callee);
+  bool IsCopyOrMove = MD && MD->isCopyAssignmentOperator();
+  IsCopyOrMove |= MD && MD->isMoveAssignmentOperator();
+
+  // We support explicit constructor calls as an MS extension. These can be
+  // constexpr by C++11 rules.
+  auto CD = dyn_cast_or_null(MD);
+  IsCopyOrMove |= CD && CD->isCopyConstructor();
+  IsCopyOrMove |= CD && CD->isMoveConstructor();
+
   // For a trivial copy or move assignment, perform an APValue copy. This is
   // essential for unions, where the operations performed by the assignment
   // operator cannot be represented as statements.
   //
   // Skip this for non-union classes with no fields; in that case, the 
defaulted
   // copy/move does not actually read the object.
-  const CXXMethodDecl *MD = dyn_cast(Callee);
-  if (MD && MD->isDefaulted() &&
+  if (MD && MD->isDefaulted() && IsCopyOrMove &&
   (MD->getParent()->isUnion() ||
(MD->isTrivial() && hasFields(MD->getParent() {
-assert(This &&
-   (MD->isCopyAssignmentOperator() || MD->isMoveAssignmentOperator()));
+assert(This);
+
 LValue RHS;
 RHS.setFrom(Info.Ctx, ArgValues[0]);
 APValue RHSValue;


Index: clang/test/CodeGenCXX/constructor-direct-call.cpp
===
--- clang/test/CodeGenCXX/constructor-direct-call.cpp
+++ clang/test/CodeGenCXX/constructor-direct-call.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple i686-pc-mingw32 -fms-extensions -Wmicrosoft %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple i686-pc-mingw32 -fms-extensions -Wmicrosoft %s -emit-llvm -o - -std=gnu++11| FileCheck %s
 
 class Test1 {
 public:
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -4184,18 +4184,27 @@
 
   CallStackFrame Frame(Info, CallLoc, Callee, This, ArgValues.data());
 
+  const CXXMethodDecl *MD = dyn_cast(Callee);
+  bool IsCopyOrMove = MD && MD->isCopyAssignmentOperator();
+  IsCopyOrMove |= MD && MD->isMoveAssignmentOperator();
+
+  // We support explicit constructor calls as an MS extension. These can be
+  // constexpr by C++11 rules.
+  auto CD = dyn_cast_or_null(MD);
+  IsCopyOrMove |= CD && CD->isCopyConstructor();
+  IsCopyOrMove |= CD && CD->isMoveConstructor();
+
   // For a trivial copy or move assignment, perform an APValue copy. This is
   // essential for unions, where the operations performed by the assignment
   // operator cannot be represented as statements.
   //
   // Skip this for non-union classes with no fields; in that case, the defaulted
   // copy/move does not actually read the object.
-  const CXXMethodDecl *MD = dyn_cast(Callee);
-  if (MD && MD->isDefaulted() &&
+  if (MD && MD->isDefaulted() && IsCopyOrMove &&
   (MD->getParent()->isUnion() ||
(MD->isTrivial() && hasFields(MD->getParent() {
-assert(This &&
-   (MD->isCopyAssignmentOperator() || MD->isMoveAssignmentOperator()));
+assert(This);
+
 LValue RHS;
 RHS.setFrom(Info.Ctx, ArgValues[0]);
 APValue RHSValue;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44512: [AAch64] Tests for ACLE FP16 macros

[Tim Northover via Phabricator via cfe-commits]

t.p.northover accepted this revision.
t.p.northover added a comment.
This revision is now accepted and ready to land.

These look right to me.


https://reviews.llvm.org/D44512



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

[PATCH] D44561: [ARM] Add HasFloat16 to TargetInfo

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added inline comments.



Comment at: include/clang/Basic/TargetInfo.h:365
 
+  /// \brief Determine whether _Float16 is supported on this target.
+  virtual bool hasFloat16Type() const { return HasFloat16; }

`_Float16` doesn't seem to be supported anywhere in Clang (`__fp16` is).

But we should probably clarify exactly what kind of support we mean here. This 
variable doesn't affect:

  * Whether __fp16 can be used at all in source.
  * Its ABI.

I'm actually slightly worried that when we document what it does affect it'll 
end up being an ARM implementation-detail.





Comment at: lib/Basic/Targets/ARM.cpp:382
   HWDiv = 0;
-  HasFullFP16 = 0;
+  HasFloat16 = 0;
 

This is now a bool rather than a bitfield, so `false` is definitely the best 
initializer (probably was before, too, but whatever).



Comment at: lib/CodeGen/CGBuiltin.cpp:3456
   case NeonTypeFlags::Float16:
-// FIXME: Only AArch64 backend can so far properly handle half types.
-// Remove else part once ARM backend support for half is complete.
-if (Arch == llvm::Triple::aarch64)
+if (Arch == llvm::Triple::aarch64 || Arch == llvm::Triple::aarch64_be ||
+HasFloat16)

This check is equivalent to setting `HasFloat16` in `AArch64TargetInfo` at the 
moment. Are you intending to change that (say by adding more uses of 
`HasFloat16`)?


https://reviews.llvm.org/D44561



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

[PATCH] D44561: [ARM] Add HasFloat16 to TargetInfo

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added inline comments.



Comment at: include/clang/Basic/TargetInfo.h:365
 
+  /// \brief Determine whether _Float16 is supported on this target.
+  virtual bool hasFloat16Type() const { return HasFloat16; }

SjoerdMeijer wrote:
> t.p.northover wrote:
> > `_Float16` doesn't seem to be supported anywhere in Clang (`__fp16` is).
> > 
> > But we should probably clarify exactly what kind of support we mean here. 
> > This variable doesn't affect:
> > 
> >   * Whether __fp16 can be used at all in source.
> >   * Its ABI.
> > 
> > I'm actually slightly worried that when we document what it does affect 
> > it'll end up being an ARM implementation-detail.
> > 
> > 
> I've added _Float16 support in Clang commit r312794: "Add _Float16 as a C/C++ 
> source language type" :-)
Oh wow, that was ages ago! I have no idea how I still had a repo that old 
hanging around. Sorry about that.


https://reviews.llvm.org/D44561



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

[PATCH] D44561: [ARM] Add HasFloat16 to TargetInfo

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added inline comments.



Comment at: include/clang/Basic/TargetInfo.h:365
 
+  /// \brief Determine whether _Float16 is supported on this target.
+  virtual bool hasFloat16Type() const { return HasFloat16; }

t.p.northover wrote:
> SjoerdMeijer wrote:
> > t.p.northover wrote:
> > > `_Float16` doesn't seem to be supported anywhere in Clang (`__fp16` is).
> > > 
> > > But we should probably clarify exactly what kind of support we mean here. 
> > > This variable doesn't affect:
> > > 
> > >   * Whether __fp16 can be used at all in source.
> > >   * Its ABI.
> > > 
> > > I'm actually slightly worried that when we document what it does affect 
> > > it'll end up being an ARM implementation-detail.
> > > 
> > > 
> > I've added _Float16 support in Clang commit r312794: "Add _Float16 as a 
> > C/C++ source language type" :-)
> Oh wow, that was ages ago! I have no idea how I still had a repo that old 
> hanging around. Sorry about that.
I'm afraid I still think this description is iffy. For a start it is actually 
still affecting `__fp16` rather than `Float16` (since that's what float16_t is 
in the NEON header). And it's still too vague about just what "support" implies.



Comment at: lib/CodeGen/CGBuiltin.cpp:3444
  NeonTypeFlags TypeFlags,
  llvm::Triple::ArchType Arch,
+ bool HasFloat16=true,

Now that you set `HasFloat16` in AArch64, I believe `Arch` is unused.


https://reviews.llvm.org/D44561



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

[PATCH] D44561: [ARM] Pass half or i16 types for NEON intrinsics

[Tim Northover via Phabricator via cfe-commits]

t.p.northover accepted this revision.
t.p.northover added a comment.
This revision is now accepted and ready to land.

Thanks. I think this looks reasonable now.


https://reviews.llvm.org/D44561



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

[PATCH] D46013: [ARM] Conform to AAPCS when passing overaligned composites as arguments

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

I'm fine with the ABI changes, but I'm not very convinced by the 
"NaturalAlignment" name.

Far from being a privileged alignment kind, it seems to take account of a 
pretty arbitrary set of modifiers. In fact, I can't help wondering if what's 
really happened is that ARM has decided to document existing GCC practice as 
the path of least resistance and scrambled for a name. This would be fine in 
ARM-specific code but could be quite misleading in the generic ASTContext. 
Personally I'd go for `ARMNaturalAlignment` and stop pretending it's something 
anyone else needs to care about (at least until some other ABI comes along that 
does).




Comment at: lib/CodeGen/TargetInfo.cpp:5055
+  Alignment = getContext().getTypeNaturalAlign(Ty);
+  Alignment = std::min(std::max(Alignment, 64u), 128u);
+} else {

I think the max/min logic is more confusing here than the alternative:

Alignment = Alignment < 128 ? 64 : 128;


https://reviews.llvm.org/D46013



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

[PATCH] D46013: [ARM] Conform to AAPCS when passing overaligned composites as arguments

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

> I'd rather not put target names in API functions. The meaning of that field 
> is pretty target independent ("alignment of type, before alignment 
> adjustments")

Except that rule only applies to aggregates. Scalar types get their alignment 
adjusted anyway I believe. The definition is ridiculously arbitrary.

> If we use it only in ARM/AArch64 we can put a comment in that sense and maybe 
> rename it to `UnadjustedAlignement` or something else more descriptive than 
> `NaturalAlignement`?

I could just about live with that (or `UnnaturalAlignment`, though perhaps best 
not).


https://reviews.llvm.org/D46013



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

[PATCH] D53348: [AArch64] Define __ELF__ for aarch64-none-elf and other similar triples.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover accepted this revision.
t.p.northover added a comment.
This revision is now accepted and ready to land.

Looks reasonable.


Repository:
  rC Clang

https://reviews.llvm.org/D53348



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

[PATCH] D53514: os_log: make buffer size an integer constant expression.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover created this revision.
Herald added a subscriber: mcrosier.

The size of an os_log buffer is known at any stage of compilation, so making it 
a constant expression means that the common idiom of declaring a buffer for it 
won't result in a VLA. That allows the compiler to skip saving and restoring 
the stack pointer around such buffers.

There can be hundreds or even thousands of such calls, even in firmware 
projects, so the code size savings add up.


Repository:
  rC Clang

https://reviews.llvm.org/D53514

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins.c


Index: clang/test/CodeGen/builtins.c
===
--- clang/test/CodeGen/builtins.c
+++ clang/test/CodeGen/builtins.c
@@ -729,25 +729,28 @@
 
 // CHECK-LABEL: define void @test_builtin_os_log_errno
 void test_builtin_os_log_errno() {
-  // CHECK: %[[VLA:.*]] = alloca i8, i64 4, align 16
-  // CHECK: call void @__os_log_helper_16_2_1_0_96(i8* %[[VLA]])
+  // CHECK-NOT: @stacksave
+  // CHECK: %[[BUF:.*]] = alloca [4 x i8], align 1
+  // CHECK: %[[DECAY:.*]] = getelementptr inbounds [4 x i8], [4 x i8]* 
%[[BUF]], i32 0, i32 0
+  // CHECK: call void @__os_log_helper_1_2_1_0_96(i8* %[[DECAY]])
+  // CHECK-NOT: @stackrestore
 
   char buf[__builtin_os_log_format_buffer_size("%m")];
   __builtin_os_log_format(buf, "%m");
 }
 
-// CHECK-LABEL: define linkonce_odr hidden void @__os_log_helper_16_2_1_0_96
+// CHECK-LABEL: define linkonce_odr hidden void @__os_log_helper_1_2_1_0_96
 // CHECK: (i8* %[[BUFFER:.*]])
 
 // CHECK: %[[BUFFER_ADDR:.*]] = alloca i8*, align 8
 // CHECK: store i8* %[[BUFFER]], i8** %[[BUFFER_ADDR]], align 8
 // CHECK: %[[BUF:.*]] = load i8*, i8** %[[BUFFER_ADDR]], align 8
 // CHECK: %[[SUMMARY:.*]] = getelementptr i8, i8* %[[BUF]], i64 0
-// CHECK: store i8 2, i8* %[[SUMMARY]], align 16
+// CHECK: store i8 2, i8* %[[SUMMARY]], align 1
 // CHECK: %[[NUMARGS:.*]] = getelementptr i8, i8* %[[BUF]], i64 1
 // CHECK: store i8 1, i8* %[[NUMARGS]], align 1
 // CHECK: %[[ARGDESCRIPTOR:.*]] = getelementptr i8, i8* %[[BUF]], i64 2
-// CHECK: store i8 96, i8* %[[ARGDESCRIPTOR]], align 2
+// CHECK: store i8 96, i8* %[[ARGDESCRIPTOR]], align 1
 // CHECK: %[[ARGSIZE:.*]] = getelementptr i8, i8* %[[BUF]], i64 3
 // CHECK: store i8 0, i8* %[[ARGSIZE]], align 1
 // CHECK-NEXT: ret void
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -3594,13 +3594,6 @@
   case Builtin::BI__builtin_os_log_format:
 return emitBuiltinOSLogFormat(*E);
 
-  case Builtin::BI__builtin_os_log_format_buffer_size: {
-analyze_os_log::OSLogBufferLayout Layout;
-analyze_os_log::computeOSLogBufferLayout(CGM.getContext(), E, Layout);
-return RValue::get(ConstantInt::get(ConvertType(E->getType()),
-Layout.size().getQuantity()));
-  }
-
   case Builtin::BI__xray_customevent: {
 if (!ShouldXRayInstrumentFunction())
   return RValue::getIgnored();
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -33,6 +33,7 @@
 //
 
//===--===//
 
+#include "clang/Analysis/Analyses/OSLog.h"
 #include "clang/AST/APValue.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTDiagnostic.h"
@@ -8112,6 +8113,12 @@
 llvm_unreachable("unexpected EvalMode");
   }
 
+  case Builtin::BI__builtin_os_log_format_buffer_size: {
+analyze_os_log::OSLogBufferLayout Layout;
+analyze_os_log::computeOSLogBufferLayout(Info.Ctx, E, Layout);
+return Success(Layout.size().getQuantity(), E);
+  }
+
   case Builtin::BI__builtin_bswap16:
   case Builtin::BI__builtin_bswap32:
   case Builtin::BI__builtin_bswap64: {


Index: clang/test/CodeGen/builtins.c
===
--- clang/test/CodeGen/builtins.c
+++ clang/test/CodeGen/builtins.c
@@ -729,25 +729,28 @@
 
 // CHECK-LABEL: define void @test_builtin_os_log_errno
 void test_builtin_os_log_errno() {
-  // CHECK: %[[VLA:.*]] = alloca i8, i64 4, align 16
-  // CHECK: call void @__os_log_helper_16_2_1_0_96(i8* %[[VLA]])
+  // CHECK-NOT: @stacksave
+  // CHECK: %[[BUF:.*]] = alloca [4 x i8], align 1
+  // CHECK: %[[DECAY:.*]] = getelementptr inbounds [4 x i8], [4 x i8]* %[[BUF]], i32 0, i32 0
+  // CHECK: call void @__os_log_helper_1_2_1_0_96(i8* %[[DECAY]])
+  // CHECK-NOT: @stackrestore
 
   char buf[__builtin_os_log_format_buffer_size("%m")];
   __builtin_os_log_format(buf, "%m");
 }
 
-// CHECK-LABEL: define linkonce_odr hidden void @__os_log_helper_16_2_1_0_96
+// CHECK-LABEL: define linkonce_odr hidden void @__os_log_helper_1_2_1_0_96
 // CHECK: (i8* %[[BUFFER:.*]])
 
 // CHECK: %[[BUFFER_ADDR:.*]

[PATCH] D53633: [AArch64] Implement FP16FML intrinsics

[Tim Northover via Phabricator via cfe-commits]

t.p.northover accepted this revision.
t.p.northover added a comment.
This revision is now accepted and ready to land.

I think this is reasonable.


Repository:
  rC Clang

https://reviews.llvm.org/D53633



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

[PATCH] D53514: os_log: make buffer size an integer constant expression.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

Ping.


Repository:
  rC Clang

https://reviews.llvm.org/D53514



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

[PATCH] D53514: os_log: make buffer size an integer constant expression.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover closed this revision.
t.p.northover added a comment.

Thanks Eli. I committed it as r345828, and then had to fixup some linker 
dependencies on other platforms, which took me a couple of tries (r345833 and 
r345835).


Repository:
  rC Clang

https://reviews.llvm.org/D53514



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

[PATCH] D53514: os_log: make buffer size an integer constant expression.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover reopened this revision.
t.p.northover added a comment.
This revision is now accepted and ready to land.

Turns out I neglected the layering between libclangAST and libclangAnalysis so 
I've reverted for now. For this to work I think we need to move the OSLog 
helpers into libclangAST. I'll put together a new patch and upload it.


Repository:
  rC Clang

https://reviews.llvm.org/D53514



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

[PATCH] D53514: os_log: make buffer size an integer constant expression.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover updated this revision to Diff 172155.
t.p.northover added a comment.
Herald added a subscriber: mgorny.

Same as previous patch except the OSLog helpers are moved to libclangAST to 
respect dependencies.


Repository:
  rC Clang

https://reviews.llvm.org/D53514

Files:
  clang/include/clang/AST/OSLog.h
  clang/include/clang/Analysis/Analyses/OSLog.h
  clang/lib/AST/CMakeLists.txt
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/OSLog.cpp
  clang/lib/Analysis/CMakeLists.txt
  clang/lib/Analysis/OSLog.cpp
  clang/lib/Analysis/PrintfFormatString.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins.c

Index: clang/test/CodeGen/builtins.c
===
--- clang/test/CodeGen/builtins.c
+++ clang/test/CodeGen/builtins.c
@@ -729,25 +729,28 @@
 
 // CHECK-LABEL: define void @test_builtin_os_log_errno
 void test_builtin_os_log_errno() {
-  // CHECK: %[[VLA:.*]] = alloca i8, i64 4, align 16
-  // CHECK: call void @__os_log_helper_16_2_1_0_96(i8* %[[VLA]])
+  // CHECK-NOT: @stacksave
+  // CHECK: %[[BUF:.*]] = alloca [4 x i8], align 1
+  // CHECK: %[[DECAY:.*]] = getelementptr inbounds [4 x i8], [4 x i8]* %[[BUF]], i32 0, i32 0
+  // CHECK: call void @__os_log_helper_1_2_1_0_96(i8* %[[DECAY]])
+  // CHECK-NOT: @stackrestore
 
   char buf[__builtin_os_log_format_buffer_size("%m")];
   __builtin_os_log_format(buf, "%m");
 }
 
-// CHECK-LABEL: define linkonce_odr hidden void @__os_log_helper_16_2_1_0_96
+// CHECK-LABEL: define linkonce_odr hidden void @__os_log_helper_1_2_1_0_96
 // CHECK: (i8* %[[BUFFER:.*]])
 
 // CHECK: %[[BUFFER_ADDR:.*]] = alloca i8*, align 8
 // CHECK: store i8* %[[BUFFER]], i8** %[[BUFFER_ADDR]], align 8
 // CHECK: %[[BUF:.*]] = load i8*, i8** %[[BUFFER_ADDR]], align 8
 // CHECK: %[[SUMMARY:.*]] = getelementptr i8, i8* %[[BUF]], i64 0
-// CHECK: store i8 2, i8* %[[SUMMARY]], align 16
+// CHECK: store i8 2, i8* %[[SUMMARY]], align 1
 // CHECK: %[[NUMARGS:.*]] = getelementptr i8, i8* %[[BUF]], i64 1
 // CHECK: store i8 1, i8* %[[NUMARGS]], align 1
 // CHECK: %[[ARGDESCRIPTOR:.*]] = getelementptr i8, i8* %[[BUF]], i64 2
-// CHECK: store i8 96, i8* %[[ARGDESCRIPTOR]], align 2
+// CHECK: store i8 96, i8* %[[ARGDESCRIPTOR]], align 1
 // CHECK: %[[ARGSIZE:.*]] = getelementptr i8, i8* %[[BUF]], i64 3
 // CHECK: store i8 0, i8* %[[ARGSIZE]], align 1
 // CHECK-NEXT: ret void
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -21,7 +21,7 @@
 #include "TargetInfo.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
-#include "clang/Analysis/Analyses/OSLog.h"
+#include "clang/AST/OSLog.h"
 #include "clang/Basic/TargetBuiltins.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/CodeGen/CGFunctionInfo.h"
@@ -3609,13 +3609,6 @@
   case Builtin::BI__builtin_os_log_format:
 return emitBuiltinOSLogFormat(*E);
 
-  case Builtin::BI__builtin_os_log_format_buffer_size: {
-analyze_os_log::OSLogBufferLayout Layout;
-analyze_os_log::computeOSLogBufferLayout(CGM.getContext(), E, Layout);
-return RValue::get(ConstantInt::get(ConvertType(E->getType()),
-Layout.size().getQuantity()));
-  }
-
   case Builtin::BI__xray_customevent: {
 if (!ShouldXRayInstrumentFunction())
   return RValue::getIgnored();
Index: clang/lib/Analysis/PrintfFormatString.cpp
===
--- clang/lib/Analysis/PrintfFormatString.cpp
+++ clang/lib/Analysis/PrintfFormatString.cpp
@@ -13,7 +13,7 @@
 //===--===//
 
 #include "clang/Analysis/Analyses/FormatString.h"
-#include "clang/Analysis/Analyses/OSLog.h"
+#include "clang/AST/OSLog.h"
 #include "FormatStringParsing.h"
 #include "clang/Basic/TargetInfo.h"
 
Index: clang/lib/Analysis/CMakeLists.txt
===
--- clang/lib/Analysis/CMakeLists.txt
+++ clang/lib/Analysis/CMakeLists.txt
@@ -18,7 +18,6 @@
   ExprMutationAnalyzer.cpp
   FormatString.cpp
   LiveVariables.cpp
-  OSLog.cpp
   ObjCNoReturn.cpp
   PostOrderCFGView.cpp
   PrintfFormatString.cpp
Index: clang/lib/AST/OSLog.cpp
===
--- clang/lib/AST/OSLog.cpp
+++ clang/lib/AST/OSLog.cpp
@@ -1,6 +1,6 @@
 // TODO: header template
 
-#include "clang/Analysis/Analyses/OSLog.h"
+#include "clang/AST/OSLog.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -39,6 +39,7 @@
 #include "clang/AST/ASTLambda.h"
 #include "clang/AST/CharUnits.h"
 #include "clang/AST/Expr.h"
+#include "clang/AST/OSLog.h"
 #include "clan

[PATCH] D54604: Automatic variable initialization

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

> This isn't meant to change the semantics of C and C++.

As I said in the cfe-dev thread, that's exactly what it does. Specifically I 
think this is essentially defining a new dialect of C++, which I have massive 
concerns about. Additionally, as much as we might claim it's a transitional 
measure, we all know that's not how it'll be used in practice.


Repository:
  rC Clang

https://reviews.llvm.org/D54604



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

[PATCH] D55562: Atomics: support min/max orthogonally

[Tim Northover via Phabricator via cfe-commits]

t.p.northover created this revision.
t.p.northover added reviewers: delena, yaxunl.
Herald added subscribers: jfb, mcrosier.

We seem to have been gradually growing support for atomic min/max operations 
(exposing longstanding IR atomicrmw instructions). But until now there have 
been gaps in the expected intrinsics. This adds support for the C11-style 
intrinsics (i.e. taking _Atomic, rather than individually blessed by C11 
standard), and the variants that return the new value instead of the original 
one.

That way, people won't be misled by trying one form and it not working, and the 
front-end is more friendly to people using _Atomic types, as we recommend.


Repository:
  rC Clang

https://reviews.llvm.org/D55562

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/Builtins.def
  clang/lib/AST/Expr.cpp
  clang/lib/CodeGen/CGAtomic.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/atomic-ops-libcall.c
  clang/test/CodeGen/atomic-ops.c
  clang/test/Sema/atomic-ops.c

Index: clang/test/Sema/atomic-ops.c
===
--- clang/test/Sema/atomic-ops.c
+++ clang/test/Sema/atomic-ops.c
@@ -354,6 +354,20 @@
   (void)__c11_atomic_fetch_xor(Ap, val, memory_order_acq_rel);
   (void)__c11_atomic_fetch_xor(Ap, val, memory_order_seq_cst);
 
+  (void)__c11_atomic_fetch_min(Ap, val, memory_order_relaxed);
+  (void)__c11_atomic_fetch_min(Ap, val, memory_order_acquire);
+  (void)__c11_atomic_fetch_min(Ap, val, memory_order_consume);
+  (void)__c11_atomic_fetch_min(Ap, val, memory_order_release);
+  (void)__c11_atomic_fetch_min(Ap, val, memory_order_acq_rel);
+  (void)__c11_atomic_fetch_min(Ap, val, memory_order_seq_cst);
+
+  (void)__c11_atomic_fetch_max(Ap, val, memory_order_relaxed);
+  (void)__c11_atomic_fetch_max(Ap, val, memory_order_acquire);
+  (void)__c11_atomic_fetch_max(Ap, val, memory_order_consume);
+  (void)__c11_atomic_fetch_max(Ap, val, memory_order_release);
+  (void)__c11_atomic_fetch_max(Ap, val, memory_order_acq_rel);
+  (void)__c11_atomic_fetch_max(Ap, val, memory_order_seq_cst);
+
   (void)__c11_atomic_exchange(Ap, val, memory_order_relaxed);
   (void)__c11_atomic_exchange(Ap, val, memory_order_acquire);
   (void)__c11_atomic_exchange(Ap, val, memory_order_consume);
@@ -501,6 +515,20 @@
   (void)__atomic_nand_fetch(p, val, memory_order_acq_rel);
   (void)__atomic_nand_fetch(p, val, memory_order_seq_cst);
 
+  (void)__atomic_max_fetch(p, val, memory_order_relaxed);
+  (void)__atomic_max_fetch(p, val, memory_order_acquire);
+  (void)__atomic_max_fetch(p, val, memory_order_consume);
+  (void)__atomic_max_fetch(p, val, memory_order_release);
+  (void)__atomic_max_fetch(p, val, memory_order_acq_rel);
+  (void)__atomic_max_fetch(p, val, memory_order_seq_cst);
+
+  (void)__atomic_min_fetch(p, val, memory_order_relaxed);
+  (void)__atomic_min_fetch(p, val, memory_order_acquire);
+  (void)__atomic_min_fetch(p, val, memory_order_consume);
+  (void)__atomic_min_fetch(p, val, memory_order_release);
+  (void)__atomic_min_fetch(p, val, memory_order_acq_rel);
+  (void)__atomic_min_fetch(p, val, memory_order_seq_cst);
+
   (void)__atomic_exchange_n(p, val, memory_order_relaxed);
   (void)__atomic_exchange_n(p, val, memory_order_acquire);
   (void)__atomic_exchange_n(p, val, memory_order_consume);
Index: clang/test/CodeGen/atomic-ops.c
===
--- clang/test/CodeGen/atomic-ops.c
+++ clang/test/CodeGen/atomic-ops.c
@@ -661,4 +661,46 @@
   __atomic_compare_exchange(&aligned_a, &aligned_b, &aligned_c, 1, memory_order_seq_cst, memory_order_seq_cst);
 }
 
+void test_c11_minmax(_Atomic(int) *si, _Atomic(unsigned) *ui) {
+  // CHECK-LABEL: @test_c11_minmax
+
+  // CHECK: atomicrmw max
+  *si = __c11_atomic_fetch_max(si, 42, memory_order_acquire);
+  // CHECK: atomicrmw min
+  *si = __c11_atomic_fetch_min(si, 42, memory_order_acquire);
+  // CHECK: atomicrmw umax
+  *ui = __c11_atomic_fetch_max(ui, 42, memory_order_acquire);
+  // CHECK: atomicrmw umin
+  *ui = __c11_atomic_fetch_min(ui, 42, memory_order_acquire);
+}
+
+void test_minmax_postop(int *si, unsigned *ui) {
+  int val = 42;
+  // CHECK-LABEL: @test_minmax_postop
+
+  // CHECK: [[OLD:%.*]] = atomicrmw max i32* [[PTR:%.*]], i32 [[RHS:%.*]] release
+  // CHECK: [[TST:%.*]] = icmp sgt i32 [[OLD]], [[RHS]]
+  // CHECK: [[NEW:%.*]] = select i1 [[TST]], i32 [[OLD]], i32 [[RHS]]
+  // CHECK: store i32 [[NEW]], i32*
+  *si = __atomic_max_fetch(si, 42, memory_order_release);
+
+  // CHECK: [[OLD:%.*]] = atomicrmw min i32* [[PTR:%.*]], i32 [[RHS:%.*]] release
+  // CHECK: [[TST:%.*]] = icmp slt i32 [[OLD]], [[RHS]]
+  // CHECK: [[NEW:%.*]] = select i1 [[TST]], i32 [[OLD]], i32 [[RHS]]
+  // CHECK: store i32 [[NEW]], i32*
+  *si = __atomic_min_fetch(si, 42, memory_order_release);
+  
+  // CHECK: [[OLD:%.*]] = atomicrmw umax i32* [[PTR:%.*]], i32 [[RHS:%.*]] release
+  // CHECK: [[TST:%.*]] = icmp ugt i32 [[OLD]], [[RHS]]
+  // CHECK

[PATCH] D55562: Atomics: support min/max orthogonally

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

> What does it do with floating-point inputs?

Same as all the other atomics: run screaming for the hills (or error out, in 
more reasonable terms). The only way to implement floating atomics in LLVM IR 
would be with a compare-exchange loop and Clang doesn't expose anything to make 
that more convenient.

Min/max have at least some pedigree in integer terms. OpenCL has specified 
them, CPUs have implemented them, and we've had requests for builtins from 
users.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55562



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

[PATCH] D59615: [AArch64] When creating SISD intrinsic calls widen scalar args into a zero vectors, not undef

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

Did you look into a scalar variant of the intrinsic call instead? These 
instructions have non-vector variants (e.g. `sqadd s0, s0, s0`), and that's 
actually why the intrinsics exist in the first place. It'd be a shame to always 
require this extra work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59615



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

[PATCH] D60485: [AArch64] Add support for MTE intrinsics

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:9566
+def err_memtag_arg_null_or_pointer : Error<
+  "%0 argument must be a null or a pointer (%1 invalid)">;
+def err_memtag_any2arg_pointer : Error<

I think these diagnostics could do with a bit more context for consistency. 
They seem to take "MTE builtin" for granted, whereas most Clang messages 
mention what they're talking about.

I'm not saying "MTE builtin" is the best we can come up with, BTW, just that 
something more would be nice.



Comment at: lib/Headers/arm_acle.h:610-615
+#define __arm_mte_create_random_tag(__ptr, __mask)  __builtin_arm_irg(__ptr, 
__mask)
+#define __arm_mte_increment_tag(__ptr, __tag_offset)  
__builtin_arm_addg(__ptr, __tag_offset)
+#define __arm_mte_exclude_tag(__ptr, __excluded)  __builtin_arm_gmi(__ptr, 
__excluded)
+#define __arm_mte_get_tag(__ptr) __builtin_arm_ldg(__ptr)
+#define __arm_mte_set_tag(__ptr) __builtin_arm_stg(__ptr)
+#define __arm_mte_ptrdiff(__ptra, __ptrb) __builtin_arm_subp(__ptra, __ptrb)

Why are the builtin names so different from the ones exposed. GCC 
compatibility? LLVM?



Comment at: lib/Sema/SemaChecking.cpp:6113
+bool Sema::SemaBuiltinARMMemoryTaggingCall( unsigned BuiltinID, CallExpr 
*TheCall) {
+  bool IsMTEBuiltin = BuiltinID == AArch64::BI__builtin_arm_irg ||
+  BuiltinID == AArch64::BI__builtin_arm_addg ||

I think this will generate an unused variable warning in a non-asserts build.


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

https://reviews.llvm.org/D60485



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

[PATCH] D60485: [AArch64] Add support for MTE intrinsics

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:9566
+def err_memtag_arg_null_or_pointer : Error<
+  "%0 argument must be a null or a pointer (%1 invalid)">;
+def err_memtag_any2arg_pointer : Error<

javed.absar wrote:
> t.p.northover wrote:
> > I think these diagnostics could do with a bit more context for consistency. 
> > They seem to take "MTE builtin" for granted, whereas most Clang messages 
> > mention what they're talking about.
> > 
> > I'm not saying "MTE builtin" is the best we can come up with, BTW, just 
> > that something more would be nice.
> I thought of doing that too , so can prefix these warnings with 'mte builtin' 
> if that's what you meant. But the intrinsic called kind of names same thing 
> (__arm_mte_..).
"Builtin" or even "function" (fixed up to be grammatical) would suffice if you 
don't like the repetition.



Comment at: lib/Headers/arm_acle.h:610-615
+#define __arm_mte_create_random_tag(__ptr, __mask)  __builtin_arm_irg(__ptr, 
__mask)
+#define __arm_mte_increment_tag(__ptr, __tag_offset)  
__builtin_arm_addg(__ptr, __tag_offset)
+#define __arm_mte_exclude_tag(__ptr, __excluded)  __builtin_arm_gmi(__ptr, 
__excluded)
+#define __arm_mte_get_tag(__ptr) __builtin_arm_ldg(__ptr)
+#define __arm_mte_set_tag(__ptr) __builtin_arm_stg(__ptr)
+#define __arm_mte_ptrdiff(__ptra, __ptrb) __builtin_arm_subp(__ptra, __ptrb)

javed.absar wrote:
> t.p.northover wrote:
> > Why are the builtin names so different from the ones exposed. GCC 
> > compatibility? LLVM?
> The builtin name (e.g. _mte_irg) is reflecting the instruction that 
> implements the otherwise meaningful ACLE names  (mte_create_tag). Its just 
> that the instruction names are sometimes cryptic (e.g. stg, ldg). I could 
> change the names to __builtin_arm_create_tag etc and push the meaningful name 
> -> insn level name to intrinsic level or further down but that would mean 
> lots of name changes to current patch and tests. 
OK, sounds reasonable to leave it as is then.



Comment at: lib/Sema/SemaChecking.cpp:6112
+/// SemaBuiltinARMMemoryTaggingCall - Handle calls of memory tagging extensions
+bool Sema::SemaBuiltinARMMemoryTaggingCall( unsigned BuiltinID, CallExpr 
*TheCall) {
+  bool IsMTEBuiltin = BuiltinID == AArch64::BI__builtin_arm_irg ||

Stray space here between '(' and 'unsigned', BTW.


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

https://reviews.llvm.org/D60485



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

[PATCH] D60709: [ARM] Support inline assembler constraints for MVE.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

Is this coordinated with GCC?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60709



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

[PATCH] D60710: [ARM] Add ACLE feature macros for MVE.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

Could you add some tests?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60710



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

[PATCH] D60691: [ARM] Replace fp-only-sp and d16 with fp64 and d32.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

I like the direction of this change, and the details look correct too.

The one thing I wonder about is whether we should be upgrading .bc files too 
(or otherwise support fp-only-sp in legacy inputs). I think it's a specialized 
enough feature that there won't be much older IR being mixed in, but can't be 
sure there's none. Anyone else got any views?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60691



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

[PATCH] D60697: [ARM] Allow "-march=foo+fp" to vary with foo.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

This needs some tests. I'm also not quite sure when you'd use bare "+fp", if 
it's the default anyway.




Comment at: llvm/lib/Support/ARMTargetParser.cpp:476
 
-StringRef ARM::getArchExtFeature(StringRef ArchExt) {
-  if (ArchExt.startswith("no")) {
-StringRef ArchExtBase(ArchExt.substr(2));
-for (const auto AE : ARCHExtNames) {
-  if (AE.NegFeature && ArchExtBase == AE.getName())
-return StringRef(AE.NegFeature);
-}
+static bool wasNegated(StringRef &Name) {
+  if (Name.startswith("no")) {

I think `isNegated` is probably more in line with existing naming.



Comment at: llvm/lib/Support/ARMTargetParser.cpp:504-507
+  if (ArchExt == "fp" || ArchExt == "fp.dp") {
+unsigned FPUKind;
+if (Negated) {
+  FPUKind = ARM::FK_NONE;

Doesn't this mean `+nofp.dp` disables all floats? That seems surprising 
behaviour to me, but I can't find any existing tests covering it.



Comment at: llvm/lib/Support/ARMTargetParser.cpp:516-517
+const FPUName &DefaultFPU = FPUNames[FPUKind];
+if (DefaultFPU.Restriction != FPURestriction::SP_D16)
+  return false;// meaningless for this arch
+

Doesn't this silently disable the FPU entirely if we decide "fp.dp" is useless? 
That seems unlikely to be what a user wants, especially without a diagnostic.

Could you also expand on the comment a bit more. I had to look up exactly what 
FPURestrictions existed to get this, and I'm not even 100% sure I'm right now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60697



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

[PATCH] D60699: [ARM] add CLI support for 8.1-M and MVE.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover accepted this revision.
t.p.northover added a comment.
This revision is now accepted and ready to land.

This looks good to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60699



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

[PATCH] D60709: [ARM] Support inline assembler constraints for MVE.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover accepted this revision.
t.p.northover added a comment.
This revision is now accepted and ready to land.

Excellent, looks good to me then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60709



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

[PATCH] D60697: [ARM] Allow "-march=foo+fp" to vary with foo.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added inline comments.



Comment at: llvm/lib/Support/ARMTargetParser.cpp:476
 
-StringRef ARM::getArchExtFeature(StringRef ArchExt) {
-  if (ArchExt.startswith("no")) {
-StringRef ArchExtBase(ArchExt.substr(2));
-for (const auto AE : ARCHExtNames) {
-  if (AE.NegFeature && ArchExtBase == AE.getName())
-return StringRef(AE.NegFeature);
-}
+static bool wasNegated(StringRef &Name) {
+  if (Name.startswith("no")) {

simon_tatham wrote:
> t.p.northover wrote:
> > I think `isNegated` is probably more in line with existing naming.
> Hmmm. I thought that would be a confusing name because it hides the fact that 
> the function strips off the `no` prefix. (The use of 'was' was intended to 
> hint that by the time the function returns, it's not true any more!)
> 
> Perhaps `stripNegationPrefix` (returning bool to indicate success)?
Ah yes, I see. I think your alternative is probably better.



Comment at: llvm/lib/Support/ARMTargetParser.cpp:504-507
+  if (ArchExt == "fp" || ArchExt == "fp.dp") {
+unsigned FPUKind;
+if (Negated) {
+  FPUKind = ARM::FK_NONE;

simon_tatham wrote:
> t.p.northover wrote:
> > Doesn't this mean `+nofp.dp` disables all floats? That seems surprising 
> > behaviour to me, but I can't find any existing tests covering it.
> Hmmm, that's a good point. What //would// a user expect in that situation? If 
> double-precision FP was the default for that architecture and a 
> single-precision version existed, then perhaps `nofp.dp` should fall back to 
> that, but what if it's double or nothing?
I think I'd go for a diagnostic in that case. There's already a way to strip 
out the FPU then (`+nofp`).



Comment at: llvm/lib/Support/ARMTargetParser.cpp:516-517
+const FPUName &DefaultFPU = FPUNames[FPUKind];
+if (DefaultFPU.Restriction != FPURestriction::SP_D16)
+  return false;// meaningless for this arch
+

simon_tatham wrote:
> t.p.northover wrote:
> > Doesn't this silently disable the FPU entirely if we decide "fp.dp" is 
> > useless? That seems unlikely to be what a user wants, especially without a 
> > diagnostic.
> > 
> > Could you also expand on the comment a bit more. I had to look up exactly 
> > what FPURestrictions existed to get this, and I'm not even 100% sure I'm 
> > right now.
> I don't think it //silently// disables it, does it? Returning false from this 
> function is a failure indication that ends up back in `checkARMArchName` in 
> `clang/lib/Driver/ToolChains/Arch/ARM.cpp`, which will generate a diagnostic. 
> For example, if I try `-march=armv6m+fp.dp` then I see
> ```
> error: the clang compiler does not support '-march=armv6m+fp.dp'
> ```
Ah, I missed the only way return true could happen and assumed the return value 
was vestigial. Sorry.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60697



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

[PATCH] D60719: Demonstrate how to fix freestanding for memcpy

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

I think it'd be pretty unpopular with the people I know who use freestanding. 
They're mostly working on microcontrollers and compiling -Oz so the extra code 
size would be untenable; they also have memcpy implementations anyway because 
they use it in their own code.

If I was trying to solve this (and I'm also not 100% sure it needs solving), I 
think I'd instead generate a `linkonce` definition of `memcpy` whenever it's 
needed and leave CodeGen unmodified.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60719



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

[PATCH] D60719: Demonstrate how to fix freestanding for memcpy

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

> IIUC freestanding environment should not rely on memcpy being present so my 
> take on it was that by "fixing" freestanding I could have my cake and eat it 
> too.

The formal situation is that freestanding implementations are only required to 
provide language support stuff like `va_list`. They're allowed to give more of 
the standard library if they want though, as implementation defined behaviour.

In practice, everyone I know provides the basic `string.h` functions and the 
compiler is pretty explicit about relying on them being present for 
correctness. They're part of the surrounding environment a user is expected to 
supply (much like the various exception handling callbacks if you want C++ 
exceptions, but always required).

For the people actually using freestanding I think they're mostly considered 
important enough that they're implemented in assembly anyway so this isn't 
really a burden, but...

> Ultimately I'm interested in implementing libc's mem function in C++. Let's 
> take memcpy for instance

Ah, that is an entirely different problem from what I thought you were trying 
to do. In that case I'm all in favour of some solution, but would start 
thinking along the lines of `-fno-builtin-memcpy` options (it's possible that 
already does what you want, but can't get LLVM to form a `memcpy` from quick 
tests to check).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60719



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

[PATCH] D60485: [AArch64] Add support for MTE intrinsics

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added inline comments.



Comment at: include/clang/Sema/Sema.h:10762
 bool AllowName);
+ bool SemaBuiltinARMMemoryTaggingCall(unsigned BuiltinID, CallExpr *TheCall);
 public:

Slightly misaligned.



Comment at: lib/CodeGen/CGBuiltin.cpp:7129-7131
+// Although it is possible to supply a different return
+// address (first arg) to this intrinsic, for now we set
+// return address same as input address.

I think this should be fixed now.  It looks like technical debt from the fact 
that the instructions only fairly recently gained that feature after the 
intrinsics were implemented internally. There's no good way to justify the 
current semantics to someone unaware of that history.


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

https://reviews.llvm.org/D60485



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

[PATCH] D60485: [AArch64] Add support for MTE intrinsics

[Tim Northover via Phabricator via cfe-commits]

t.p.northover accepted this revision.
t.p.northover added inline comments.
This revision is now accepted and ready to land.



Comment at: lib/CodeGen/CGBuiltin.cpp:7129-7131
+// Although it is possible to supply a different return
+// address (first arg) to this intrinsic, for now we set
+// return address same as input address.

javed.absar wrote:
> t.p.northover wrote:
> > I think this should be fixed now.  It looks like technical debt from the 
> > fact that the instructions only fairly recently gained that feature after 
> > the intrinsics were implemented internally. There's no good way to justify 
> > the current semantics to someone unaware of that history.
> Not quite that really.  So the instruction did gain the feature recently like 
> you mentioned. But the ACLE/intrinsics were designed and agreed upon after it 
> and it was decided in ACLE discussions that the exta feature added complexity 
> that need not be exposed at ACLE level yet. No big use case to justify 
> complicating the ACLE MTE spec yet. Directly assembly can use that 
> instruction though.
I think the ACLE decision was a mistake, but since it happened I withdraw this 
request. I expect (and hope) far more people will use these through ACLE than 
as compiler-specific builtins.


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

https://reviews.llvm.org/D60485



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

[PATCH] D55562: Atomics: support min/max orthogonally

[Tim Northover via Phabricator via cfe-commits]

t.p.northover updated this revision to Diff 198452.
t.p.northover added a comment.

Sorry, I managed to forget about this one somehow. I hadn't changed the 32-bit 
requirement, but I agree it shouldn't be there so this diff removes it and adds 
tests for the newly legal cases.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55562

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/Expr.cpp
  clang/lib/CodeGen/CGAtomic.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/atomic-ops.c
  clang/test/Sema/atomic-ops.c
  clang/test/SemaOpenCL/atomic-ops.cl

Index: clang/test/SemaOpenCL/atomic-ops.cl
===
--- clang/test/SemaOpenCL/atomic-ops.cl
+++ clang/test/SemaOpenCL/atomic-ops.cl
@@ -73,7 +73,7 @@
   __opencl_atomic_fetch_add(d, 1, memory_order_seq_cst, memory_scope_work_group); // expected-error {{address argument to atomic operation must be a pointer to atomic integer or pointer ('__generic atomic_float *' (aka '__generic _Atomic(float) *') invalid)}}
   __opencl_atomic_fetch_and(i, 1, memory_order_seq_cst, memory_scope_work_group);
   __opencl_atomic_fetch_and(p, 1, memory_order_seq_cst, memory_scope_work_group);
-  __opencl_atomic_fetch_and(d, 1, memory_order_seq_cst, memory_scope_work_group); // expected-error {{address argument to bitwise atomic operation must be a pointer to atomic integer ('__generic atomic_float *' (aka '__generic _Atomic(float) *') invalid)}}
+  __opencl_atomic_fetch_and(d, 1, memory_order_seq_cst, memory_scope_work_group); // expected-error {{address argument to atomic operation must be a pointer to atomic integer ('__generic atomic_float *' (aka '__generic _Atomic(float) *') invalid)}}
 
   __opencl_atomic_fetch_min(i, 1, memory_order_seq_cst, memory_scope_work_group);
   __opencl_atomic_fetch_max(i, 1, memory_order_seq_cst, memory_scope_work_group);
Index: clang/test/Sema/atomic-ops.c
===
--- clang/test/Sema/atomic-ops.c
+++ clang/test/Sema/atomic-ops.c
@@ -173,8 +173,8 @@
   __atomic_fetch_sub(P, 3, memory_order_seq_cst);
   __atomic_fetch_sub(D, 3, memory_order_seq_cst); // expected-error {{must be a pointer to integer or pointer}}
   __atomic_fetch_sub(s1, 3, memory_order_seq_cst); // expected-error {{must be a pointer to integer or pointer}}
-  __atomic_fetch_min(D, 3, memory_order_seq_cst); // expected-error {{must be a pointer to signed or unsigned 32-bit integer}}
-  __atomic_fetch_max(P, 3, memory_order_seq_cst); // expected-error {{must be a pointer to signed or unsigned 32-bit integer}}
+  __atomic_fetch_min(D, 3, memory_order_seq_cst); // expected-error {{must be a pointer to integer}}
+  __atomic_fetch_max(P, 3, memory_order_seq_cst); // expected-error {{must be a pointer to integer}}
   __atomic_fetch_max(p, 3);   // expected-error {{too few arguments to function call, expected 3, have 2}}
 
   __c11_atomic_fetch_and(i, 1, memory_order_seq_cst);
@@ -354,6 +354,20 @@
   (void)__c11_atomic_fetch_xor(Ap, val, memory_order_acq_rel);
   (void)__c11_atomic_fetch_xor(Ap, val, memory_order_seq_cst);
 
+  (void)__c11_atomic_fetch_min(Ap, val, memory_order_relaxed);
+  (void)__c11_atomic_fetch_min(Ap, val, memory_order_acquire);
+  (void)__c11_atomic_fetch_min(Ap, val, memory_order_consume);
+  (void)__c11_atomic_fetch_min(Ap, val, memory_order_release);
+  (void)__c11_atomic_fetch_min(Ap, val, memory_order_acq_rel);
+  (void)__c11_atomic_fetch_min(Ap, val, memory_order_seq_cst);
+
+  (void)__c11_atomic_fetch_max(Ap, val, memory_order_relaxed);
+  (void)__c11_atomic_fetch_max(Ap, val, memory_order_acquire);
+  (void)__c11_atomic_fetch_max(Ap, val, memory_order_consume);
+  (void)__c11_atomic_fetch_max(Ap, val, memory_order_release);
+  (void)__c11_atomic_fetch_max(Ap, val, memory_order_acq_rel);
+  (void)__c11_atomic_fetch_max(Ap, val, memory_order_seq_cst);
+
   (void)__c11_atomic_exchange(Ap, val, memory_order_relaxed);
   (void)__c11_atomic_exchange(Ap, val, memory_order_acquire);
   (void)__c11_atomic_exchange(Ap, val, memory_order_consume);
@@ -501,6 +515,20 @@
   (void)__atomic_nand_fetch(p, val, memory_order_acq_rel);
   (void)__atomic_nand_fetch(p, val, memory_order_seq_cst);
 
+  (void)__atomic_max_fetch(p, val, memory_order_relaxed);
+  (void)__atomic_max_fetch(p, val, memory_order_acquire);
+  (void)__atomic_max_fetch(p, val, memory_order_consume);
+  (void)__atomic_max_fetch(p, val, memory_order_release);
+  (void)__atomic_max_fetch(p, val, memory_order_acq_rel);
+  (void)__atomic_max_fetch(p, val, memory_order_seq_cst);
+
+  (void)__atomic_min_fetch(p, val, memory_order_relaxed);
+  (void)__atomic_min_fetch(p, val, memory_order_acquire);
+  (void)__atomic_min_fetch(p, val, memory_order_consume);
+  (void)__atomic_min_fetch(p, val, memory_order_rel

[PATCH] D61939: AArch64: add support for arm64_23 (ILP32) IR generation

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

Ping.


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

https://reviews.llvm.org/D61939



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

[PATCH] D61939: AArch64: add support for arm64_23 (ILP32) IR generation

[Tim Northover via Phabricator via cfe-commits]

t.p.northover marked an inline comment as done.
t.p.northover added a comment.

Thanks for the suggestion Florian, and sorry it's taken so long to act on it. 
I've split the patch up as you suggest, I'll make this one cover the Triple 
bits.




Comment at: clang/lib/Basic/Targets/AArch64.h:93
+
+  bool hasInt128Type() const override;
 };

fhahn wrote:
> Why is this needed? It seems unused in the diff?
It's overriding a virtual function used to decide whether __int128 is allowed, 
and the default implementation checks the pointer width.

We're still AArch64 though so we can support __int128 just as well with 32-bit 
or 64-bit pointers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61939



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

[PATCH] D61939: AArch64: add support for arm64_23 (ILP32) IR generation

[Tim Northover via Phabricator via cfe-commits]

t.p.northover updated this revision to Diff 204009.
t.p.northover added a comment.

This diff now only covers the trivial additions so that "arm64_32" is 
understood by the driver and creates AArch64 instantiations of relevant 
classes. Code generated is still wildly incorrect (not even ILP32 yet).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61939

Files:
  clang/lib/Basic/Targets.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/aarch64-cpus.c
  clang/test/Driver/arm64_32-link.c
  clang/test/Preprocessor/aarch64-target-features.c
  clang/test/Sema/types.c

Index: clang/test/Sema/types.c
===
--- clang/test/Sema/types.c
+++ clang/test/Sema/types.c
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 %s -fblocks -pedantic -verify -triple=mips64-linux-gnu
 // RUN: %clang_cc1 %s -fblocks -pedantic -verify -triple=x86_64-unknown-linux
 // RUN: %clang_cc1 %s -fblocks -pedantic -verify -triple=x86_64-unknown-linux-gnux32
+// RUN: %clang_cc1 %s -fblocks -pedantic -pedantic -verify -triple=arm64_32-apple-ios7.0
 
 // rdar://6097662
 typedef int (*T)[2];
Index: clang/test/Preprocessor/aarch64-target-features.c
===
--- clang/test/Preprocessor/aarch64-target-features.c
+++ clang/test/Preprocessor/aarch64-target-features.c
@@ -169,6 +169,9 @@
 // RUN: %clang -target x86_64-apple-macosx -arch arm64 -### -c %s 2>&1 | FileCheck --check-prefix=CHECK-ARCH-ARM64 %s
 // CHECK-ARCH-ARM64: "-target-cpu" "cyclone" "-target-feature" "+fp-armv8" "-target-feature" "+neon" "-target-feature" "+crypto" "-target-feature" "+zcm" "-target-feature" "+zcz"
 
+// RUN: %clang -target x86_64-apple-macosx -arch arm64_32 -### -c %s 2>&1 | FileCheck --check-prefix=CHECK-ARCH-ARM64_32 %s
+// CHECK-ARCH-ARM64_32: "-target-cpu" "cyclone" "-target-feature" "+fp-armv8" "-target-feature" "+neon" "-target-feature" "+crypto" "-target-feature" "+zcm" "-target-feature" "+zcz"
+
 // RUN: %clang -target aarch64 -march=armv8-a+fp+simd+crc+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-MARCH-1 %s
 // RUN: %clang -target aarch64 -march=armv8-a+nofp+nosimd+nocrc+nocrypto+fp+simd+crc+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-MARCH-1 %s
 // RUN: %clang -target aarch64 -march=armv8-a+nofp+nosimd+nocrc+nocrypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-MARCH-2 %s
Index: clang/test/Driver/arm64_32-link.c
===
--- /dev/null
+++ clang/test/Driver/arm64_32-link.c
@@ -0,0 +1,4 @@
+// RUN: %clang -target x86_64-apple-darwin -arch arm64_32 -miphoneos-version-min=8.0 %s -### 2>&1 | FileCheck %s
+
+// CHECK: clang{{.*}} "-triple" "aarch64_32-apple-ios8.0.0"
+// CHECK: ld{{.*}} "-arch" "arm64_32"
Index: clang/test/Driver/aarch64-cpus.c
===
--- clang/test/Driver/aarch64-cpus.c
+++ clang/test/Driver/aarch64-cpus.c
@@ -26,6 +26,9 @@
 // ARM64-DARWIN: "-cc1"{{.*}} "-triple" "arm64{{.*}}" "-target-cpu" "cyclone"
 // ARM64-DARWIN-SAME: "-target-feature" "+aes"
 
+// RUN: %clang -target arm64-apple-darwin -arch arm64_32 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64_32-DARWIN %s
+// ARM64_32-DARWIN: "-cc1"{{.*}} "-triple" "aarch64_32{{.*}}" "-target-cpu" "cyclone"
+
 // RUN: %clang -target aarch64 -mcpu=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35 %s
 // RUN: %clang -target aarch64 -mlittle-endian -mcpu=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35 %s
 // RUN: %clang -target aarch64_be -mlittle-endian -mcpu=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35 %s
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -56,7 +56,8 @@
   .Cases("arm", "armv4t", "armv5", "armv6", "armv6m", llvm::Triple::arm)
   .Cases("armv7", "armv7em", "armv7k", "armv7m", llvm::Triple::arm)
   .Cases("armv7s", "xscale", llvm::Triple::arm)
-  .Case("arm64", llvm::Triple::aarch64)
+  .Case("arm64",  llvm::Triple::aarch64)
+  .Case("arm64_32", llvm::Triple::aarch64_32)
   .Case("r600", llvm::Triple::r600)
   .Case("amdgcn", llvm::Triple::amdgcn)
   .Case("nvptx", llvm::Triple::nvptx)
@@ -816,6 +817,9 @@
   default:
 return getDefaultUniversalArchName();
 
+  case llvm::Triple::aarch64_32:
+return "arm64_32";
+
   case llvm::Triple::aarch64:
 return "arm64";
 
@@ -1597,7 +1601,7 @@
   if (MachOArchName == "armv7" || MachOArchName == "armv7s" ||
   MachOArchName == "arm64")
 OSTy = llvm::Triple::IOS;
-  else if (MachOArchName == "armv7k")
+  else if (MachOArchName == "armv7k" || MachOArchName == "a

[PATCH] D63131: arm64_32: implement the desired ABI for the ILP32 triple.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover created this revision.
t.p.northover added a reviewer: fhahn.
Herald added subscribers: jfb, kristof.beyls, javed.absar, mcrosier.
Herald added a project: clang.

This adds all of the ABI tweaks we need to match the arm64_32 ABI as it exists 
in the wild. Most cirtically, of course, it makes the triple ILP32. But it also 
covers the usual suspects when defining an ABI.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63131

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/arm64_32-vaarg.c
  clang/test/CodeGen/arm64_32.c
  clang/test/CodeGen/builtins-arm64.c
  clang/test/CodeGen/target-data.c
  clang/test/CodeGenCXX/armv7k.cpp
  clang/test/Preprocessor/arm64_32.c
  clang/test/Preprocessor/init-v7k-compat.c
  clang/test/Preprocessor/stdint.c
  clang/test/Sema/aarch64-neon-vector-types.c

Index: clang/test/Sema/aarch64-neon-vector-types.c
===
--- clang/test/Sema/aarch64-neon-vector-types.c
+++ clang/test/Sema/aarch64-neon-vector-types.c
@@ -3,6 +3,8 @@
 // RUN: %clang_cc1 %s -triple arm64-none-linux-gnu -target-feature +neon -fsyntax-only -verify
 // RUN: %clang_cc1 %s -triple arm64-none-linux-gnu -target-feature +neon -DUSE_LONG -fsyntax-only -verify
 
+// RUN: %clang_cc1 %s -triple arm64_32-apple-ios -target-feature +neon -fsyntax-only -verify
+
 typedef float float32_t;
 typedef unsigned char poly8_t;
 typedef unsigned short poly16_t;
Index: clang/test/Preprocessor/stdint.c
===
--- clang/test/Preprocessor/stdint.c
+++ clang/test/Preprocessor/stdint.c
@@ -105,6 +105,113 @@
 // ARM:INTMAX_C_(0) 0LL
 // ARM:UINTMAX_C_(0) 0ULL
 //
+// RUN: %clang_cc1 -E -ffreestanding -triple=arm64_32-apple-ios7.0 %s | FileCheck -check-prefix ARM64_32 %s
+//
+// ARM64_32:typedef long long int int64_t;
+// ARM64_32:typedef long long unsigned int uint64_t;
+// ARM64_32:typedef int64_t int_least64_t;
+// ARM64_32:typedef uint64_t uint_least64_t;
+// ARM64_32:typedef int64_t int_fast64_t;
+// ARM64_32:typedef uint64_t uint_fast64_t;
+//
+// ARM64_32:typedef int int32_t;
+// ARM64_32:typedef unsigned int uint32_t;
+// ARM64_32:typedef int32_t int_least32_t;
+// ARM64_32:typedef uint32_t uint_least32_t;
+// ARM64_32:typedef int32_t int_fast32_t;
+// ARM64_32:typedef uint32_t uint_fast32_t;
+// 
+// ARM64_32:typedef short int16_t;
+// ARM64_32:typedef unsigned short uint16_t;
+// ARM64_32:typedef int16_t int_least16_t;
+// ARM64_32:typedef uint16_t uint_least16_t;
+// ARM64_32:typedef int16_t int_fast16_t;
+// ARM64_32:typedef uint16_t uint_fast16_t;
+//
+// ARM64_32:typedef signed char int8_t;
+// ARM64_32:typedef unsigned char uint8_t;
+// ARM64_32:typedef int8_t int_least8_t;
+// ARM64_32:typedef uint8_t uint_least8_t;
+// ARM64_32:typedef int8_t int_fast8_t;
+// ARM64_32:typedef uint8_t uint_fast8_t;
+//
+// ARM64_32:typedef long int intptr_t;
+// ARM64_32:typedef long unsigned int uintptr_t;
+// 
+// ARM64_32:typedef long long int intmax_t;
+// ARM64_32:typedef long long unsigned int uintmax_t;
+//
+// ARM64_32:INT8_MAX_ 127
+// ARM64_32:INT8_MIN_ (-127 -1)
+// ARM64_32:UINT8_MAX_ 255
+// ARM64_32:INT_LEAST8_MIN_ (-127 -1)
+// ARM64_32:INT_LEAST8_MAX_ 127
+// ARM64_32:UINT_LEAST8_MAX_ 255
+// ARM64_32:INT_FAST8_MIN_ (-127 -1)
+// ARM64_32:INT_FAST8_MAX_ 127
+// ARM64_32:UINT_FAST8_MAX_ 255
+//
+// ARM64_32:INT16_MAX_ 32767
+// ARM64_32:INT16_MIN_ (-32767 -1)
+// ARM64_32:UINT16_MAX_ 65535
+// ARM64_32:INT_LEAST16_MIN_ (-32767 -1)
+// ARM64_32:INT_LEAST16_MAX_ 32767
+// ARM64_32:UINT_LEAST16_MAX_ 65535
+// ARM64_32:INT_FAST16_MIN_ (-32767 -1)
+// ARM64_32:INT_FAST16_MAX_ 32767
+// ARM64_32:UINT_FAST16_MAX_ 65535
+//
+// ARM64_32:INT32_MAX_ 2147483647
+// ARM64_32:INT32_MIN_ (-2147483647 -1)
+// ARM64_32:UINT32_MAX_ 4294967295U
+// ARM64_32:INT_LEAST32_MIN_ (-2147483647 -1)
+// ARM64_32:INT_LEAST32_MAX_ 2147483647
+// ARM64_32:UINT_LEAST32_MAX_ 4294967295U
+// ARM64_32:INT_FAST32_MIN_ (-2147483647 -1)
+// ARM64_32:INT_FAST32_MAX_ 2147483647
+// ARM64_32:UINT_FAST32_MAX_ 4294967295U
+//
+// ARM64_32:INT64_MAX_ 9223372036854775807LL
+// ARM64_32:INT64_MIN_ (-9223372036854775807LL -1)
+// ARM64_32:UINT64_MAX_ 18446744073709551615ULL
+// ARM64_32:INT_LEAST64_MIN_ (-9223372036854775807LL -1)
+// ARM64_32:INT_LEAST64_MAX_ 9223372036854775807LL
+// ARM64_32:UINT_LEAST64_MAX_ 18446744073709551615ULL
+// ARM64_32:INT_FAST64_MIN_ (-9223372036854775807LL -1)
+// ARM64_32:INT_FAST64_MAX_ 9223372036854775807LL
+// ARM64_32:UINT_FAST64_MAX_ 18446744073709551615ULL
+//
+// ARM64_32:INTPTR_MIN_ (-2147483647L -1)
+// ARM64_32:INTPTR_MAX_ 2147483647L
+// ARM64_32:UINTPTR_MAX_ 4294967295UL
+// ARM64_32:PTRDIFF_MIN_ (-2147483647L -1)
+// ARM64_32:PTRDIFF_MAX_ 2147483647L
+// ARM64_32:SIZE_MAX_ 4294967295UL
+//

[PATCH] D64416: [AArch64] Add support for Transactional Memory Extension (TME)

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added inline comments.



Comment at: clang/test/Sema/aarch64-tme-errors.c:1
+// RUN: %clang_cc1 -triple aarch64-eabi -verify %s
+

I don't think the Sema checks need to be split over so many files. One for the 
whole of transactional seems enough. Similarly for CodeGen really.



Comment at: llvm/test/CodeGen/AArch64/tme-tcancel.ll:6
+define void @test_tcancel() #0 {
+  tail call void @llvm.aarch64.tcancel(i64 0) #1
+  ret void

Testing more values than 0 would be a good idea.


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

https://reviews.llvm.org/D64416



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

[PATCH] D65582: IR: accept and print numbered %N names for function args

[Tim Northover via Phabricator via cfe-commits]

t.p.northover accepted this revision.
t.p.northover marked an inline comment as done.
t.p.northover added a comment.

Thanks Reid, committed as r367755.




Comment at: llvm/utils/add_argument_names.py:5
+def fix_string(s):
+TYPE = 
re.compile('\s*(i[0-9]+|float|double|x86_fp80|fp128|ppc_fp128|\[\[.*?\]\]|\[2 x 
\[\[[A-Z_0-9]+\]\]\]|<.*?>|{.*?}|\[[0-9]+ x 
.*?\]|%["a-z:A-Z0-9._]+({{.*?}})?|%{{.*?}}|{{.*?}}|\[\[.*?\]\])(\s*(\*|addrspace\(.*?\)|dereferenceable\(.*?\)|byval\(.*?\)|sret|zeroext|inreg|returned|signext|nocapture|align
 \d+|swiftself|swifterror|readonly|noalias|inalloca|nocapture))*\s*')
+

rnk wrote:
> Nice.
Incoming prize for the most egregious violation of the 70 column rule.


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

https://reviews.llvm.org/D65582



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

[PATCH] D55562: Atomics: support min/max orthogonally

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

Ping.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55562



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

[PATCH] D61939: AArch64: add support for arm64_23 (ILP32) IR generation

[Tim Northover via Phabricator via cfe-commits]

t.p.northover created this revision.
Herald added subscribers: jfb, kristof.beyls, javed.absar, mcrosier.
Herald added a project: clang.

This patch implements the arm64_32 ABI used in watchOS from the Clang side. 
It's mostly pretty straightforward since it's so close to normal AArch64: 
handle the aarch64_32 Triple component and toggle the appropriate ABI flags for 
bitfields and vectors.


Repository:
  rC Clang

https://reviews.llvm.org/D61939

Files:
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/arm64_32-vaarg.c
  clang/test/CodeGen/arm64_32.c
  clang/test/CodeGen/builtins-arm64.c
  clang/test/CodeGen/target-data.c
  clang/test/CodeGenCXX/armv7k.cpp
  clang/test/Driver/aarch64-cpus.c
  clang/test/Driver/arm64_32-link.c
  clang/test/Preprocessor/aarch64-target-features.c
  clang/test/Preprocessor/arm64_32.c
  clang/test/Preprocessor/init-v7k-compat.c
  clang/test/Preprocessor/stdint.c
  clang/test/Sema/types.c

Index: clang/test/Sema/types.c
===
--- clang/test/Sema/types.c
+++ clang/test/Sema/types.c
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 %s -fblocks -pedantic -verify -triple=mips64-linux-gnu
 // RUN: %clang_cc1 %s -fblocks -pedantic -verify -triple=x86_64-unknown-linux
 // RUN: %clang_cc1 %s -fblocks -pedantic -verify -triple=x86_64-unknown-linux-gnux32
+// RUN: %clang_cc1 %s -fblocks -pedantic -pedantic -verify -triple=arm64_32-apple-ios7.0
 
 // rdar://6097662
 typedef int (*T)[2];
Index: clang/test/Preprocessor/stdint.c
===
--- clang/test/Preprocessor/stdint.c
+++ clang/test/Preprocessor/stdint.c
@@ -105,6 +105,113 @@
 // ARM:INTMAX_C_(0) 0LL
 // ARM:UINTMAX_C_(0) 0ULL
 //
+// RUN: %clang_cc1 -E -ffreestanding -triple=arm64_32-apple-ios7.0 %s | FileCheck -check-prefix ARM64_32 %s
+//
+// ARM64_32:typedef long long int int64_t;
+// ARM64_32:typedef long long unsigned int uint64_t;
+// ARM64_32:typedef int64_t int_least64_t;
+// ARM64_32:typedef uint64_t uint_least64_t;
+// ARM64_32:typedef int64_t int_fast64_t;
+// ARM64_32:typedef uint64_t uint_fast64_t;
+//
+// ARM64_32:typedef int int32_t;
+// ARM64_32:typedef unsigned int uint32_t;
+// ARM64_32:typedef int32_t int_least32_t;
+// ARM64_32:typedef uint32_t uint_least32_t;
+// ARM64_32:typedef int32_t int_fast32_t;
+// ARM64_32:typedef uint32_t uint_fast32_t;
+// 
+// ARM64_32:typedef short int16_t;
+// ARM64_32:typedef unsigned short uint16_t;
+// ARM64_32:typedef int16_t int_least16_t;
+// ARM64_32:typedef uint16_t uint_least16_t;
+// ARM64_32:typedef int16_t int_fast16_t;
+// ARM64_32:typedef uint16_t uint_fast16_t;
+//
+// ARM64_32:typedef signed char int8_t;
+// ARM64_32:typedef unsigned char uint8_t;
+// ARM64_32:typedef int8_t int_least8_t;
+// ARM64_32:typedef uint8_t uint_least8_t;
+// ARM64_32:typedef int8_t int_fast8_t;
+// ARM64_32:typedef uint8_t uint_fast8_t;
+//
+// ARM64_32:typedef long int intptr_t;
+// ARM64_32:typedef long unsigned int uintptr_t;
+// 
+// ARM64_32:typedef long long int intmax_t;
+// ARM64_32:typedef long long unsigned int uintmax_t;
+//
+// ARM64_32:INT8_MAX_ 127
+// ARM64_32:INT8_MIN_ (-127 -1)
+// ARM64_32:UINT8_MAX_ 255
+// ARM64_32:INT_LEAST8_MIN_ (-127 -1)
+// ARM64_32:INT_LEAST8_MAX_ 127
+// ARM64_32:UINT_LEAST8_MAX_ 255
+// ARM64_32:INT_FAST8_MIN_ (-127 -1)
+// ARM64_32:INT_FAST8_MAX_ 127
+// ARM64_32:UINT_FAST8_MAX_ 255
+//
+// ARM64_32:INT16_MAX_ 32767
+// ARM64_32:INT16_MIN_ (-32767 -1)
+// ARM64_32:UINT16_MAX_ 65535
+// ARM64_32:INT_LEAST16_MIN_ (-32767 -1)
+// ARM64_32:INT_LEAST16_MAX_ 32767
+// ARM64_32:UINT_LEAST16_MAX_ 65535
+// ARM64_32:INT_FAST16_MIN_ (-32767 -1)
+// ARM64_32:INT_FAST16_MAX_ 32767
+// ARM64_32:UINT_FAST16_MAX_ 65535
+//
+// ARM64_32:INT32_MAX_ 2147483647
+// ARM64_32:INT32_MIN_ (-2147483647 -1)
+// ARM64_32:UINT32_MAX_ 4294967295U
+// ARM64_32:INT_LEAST32_MIN_ (-2147483647 -1)
+// ARM64_32:INT_LEAST32_MAX_ 2147483647
+// ARM64_32:UINT_LEAST32_MAX_ 4294967295U
+// ARM64_32:INT_FAST32_MIN_ (-2147483647 -1)
+// ARM64_32:INT_FAST32_MAX_ 2147483647
+// ARM64_32:UINT_FAST32_MAX_ 4294967295U
+//
+// ARM64_32:INT64_MAX_ 9223372036854775807LL
+// ARM64_32:INT64_MIN_ (-9223372036854775807LL -1)
+// ARM64_32:UINT64_MAX_ 18446744073709551615ULL
+// ARM64_32:INT_LEAST64_MIN_ (-9223372036854775807LL -1)
+// ARM64_32:INT_LEAST64_MAX_ 9223372036854775807LL
+// ARM64_32:UINT_LEAST64_MAX_ 18446744073709551615ULL
+// ARM64_32:INT_FAST64_MIN_ (-9223372036854775807LL -1)
+// ARM64_32:INT_FAST64_MAX_ 9223372036854775807LL
+// ARM64_32:UINT_FAST64_MAX_ 18446744073709551615ULL
+//
+// ARM64_32:INTPTR_MIN_ (-214748364

[PATCH] D61939: AArch64: add support for arm64_23 (ILP32) IR generation

[Tim Northover via Phabricator via cfe-commits]

t.p.northover updated this revision to Diff 200707.
t.p.northover added a comment.

During upstreaming we've changed from detecting an "arm64_32" ArchName to using 
a Triple::aarch64_32 Arch. We recently discovered a bug that meant only AArch32 
NEON types were permitted, which is fixed in this new revision.

Also, ping.


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

https://reviews.llvm.org/D61939

Files:
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/arm64_32-vaarg.c
  clang/test/CodeGen/arm64_32.c
  clang/test/CodeGen/builtins-arm64.c
  clang/test/CodeGen/target-data.c
  clang/test/CodeGenCXX/armv7k.cpp
  clang/test/Driver/aarch64-cpus.c
  clang/test/Driver/arm64_32-link.c
  clang/test/Preprocessor/aarch64-target-features.c
  clang/test/Preprocessor/arm64_32.c
  clang/test/Preprocessor/init-v7k-compat.c
  clang/test/Preprocessor/stdint.c
  clang/test/Sema/aarch64-neon-vector-types.c
  clang/test/Sema/types.c

Index: clang/test/Sema/types.c
===
--- clang/test/Sema/types.c
+++ clang/test/Sema/types.c
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 %s -fblocks -pedantic -verify -triple=mips64-linux-gnu
 // RUN: %clang_cc1 %s -fblocks -pedantic -verify -triple=x86_64-unknown-linux
 // RUN: %clang_cc1 %s -fblocks -pedantic -verify -triple=x86_64-unknown-linux-gnux32
+// RUN: %clang_cc1 %s -fblocks -pedantic -pedantic -verify -triple=arm64_32-apple-ios7.0
 
 // rdar://6097662
 typedef int (*T)[2];
Index: clang/test/Sema/aarch64-neon-vector-types.c
===
--- clang/test/Sema/aarch64-neon-vector-types.c
+++ clang/test/Sema/aarch64-neon-vector-types.c
@@ -3,6 +3,8 @@
 // RUN: %clang_cc1 %s -triple arm64-none-linux-gnu -target-feature +neon -fsyntax-only -verify
 // RUN: %clang_cc1 %s -triple arm64-none-linux-gnu -target-feature +neon -DUSE_LONG -fsyntax-only -verify
 
+// RUN: %clang_cc1 %s -triple arm64_32-apple-ios -target-feature +neon -fsyntax-only -verify
+
 typedef float float32_t;
 typedef unsigned char poly8_t;
 typedef unsigned short poly16_t;
Index: clang/test/Preprocessor/stdint.c
===
--- clang/test/Preprocessor/stdint.c
+++ clang/test/Preprocessor/stdint.c
@@ -105,6 +105,113 @@
 // ARM:INTMAX_C_(0) 0LL
 // ARM:UINTMAX_C_(0) 0ULL
 //
+// RUN: %clang_cc1 -E -ffreestanding -triple=arm64_32-apple-ios7.0 %s | FileCheck -check-prefix ARM64_32 %s
+//
+// ARM64_32:typedef long long int int64_t;
+// ARM64_32:typedef long long unsigned int uint64_t;
+// ARM64_32:typedef int64_t int_least64_t;
+// ARM64_32:typedef uint64_t uint_least64_t;
+// ARM64_32:typedef int64_t int_fast64_t;
+// ARM64_32:typedef uint64_t uint_fast64_t;
+//
+// ARM64_32:typedef int int32_t;
+// ARM64_32:typedef unsigned int uint32_t;
+// ARM64_32:typedef int32_t int_least32_t;
+// ARM64_32:typedef uint32_t uint_least32_t;
+// ARM64_32:typedef int32_t int_fast32_t;
+// ARM64_32:typedef uint32_t uint_fast32_t;
+// 
+// ARM64_32:typedef short int16_t;
+// ARM64_32:typedef unsigned short uint16_t;
+// ARM64_32:typedef int16_t int_least16_t;
+// ARM64_32:typedef uint16_t uint_least16_t;
+// ARM64_32:typedef int16_t int_fast16_t;
+// ARM64_32:typedef uint16_t uint_fast16_t;
+//
+// ARM64_32:typedef signed char int8_t;
+// ARM64_32:typedef unsigned char uint8_t;
+// ARM64_32:typedef int8_t int_least8_t;
+// ARM64_32:typedef uint8_t uint_least8_t;
+// ARM64_32:typedef int8_t int_fast8_t;
+// ARM64_32:typedef uint8_t uint_fast8_t;
+//
+// ARM64_32:typedef long int intptr_t;
+// ARM64_32:typedef long unsigned int uintptr_t;
+// 
+// ARM64_32:typedef long long int intmax_t;
+// ARM64_32:typedef long long unsigned int uintmax_t;
+//
+// ARM64_32:INT8_MAX_ 127
+// ARM64_32:INT8_MIN_ (-127 -1)
+// ARM64_32:UINT8_MAX_ 255
+// ARM64_32:INT_LEAST8_MIN_ (-127 -1)
+// ARM64_32:INT_LEAST8_MAX_ 127
+// ARM64_32:UINT_LEAST8_MAX_ 255
+// ARM64_32:INT_FAST8_MIN_ (-127 -1)
+// ARM64_32:INT_FAST8_MAX_ 127
+// ARM64_32:UINT_FAST8_MAX_ 255
+//
+// ARM64_32:INT16_MAX_ 32767
+// ARM64_32:INT16_MIN_ (-32767 -1)
+// ARM64_32:UINT16_MAX_ 65535
+// ARM64_32:INT_LEAST16_MIN_ (-32767 -1)
+// ARM64_32:INT_LEAST16_MAX_ 32767
+// ARM64_32:UINT_LEAST16_MAX_ 65535
+// ARM64_32:INT_FAST16_MIN_ (-32767 -1)
+// ARM64_32:INT_FAST16_MAX_ 32767
+// ARM64_32:UINT_FAST16_MAX_ 65535
+//
+// ARM64_32:INT32_MAX_ 2147483647
+// ARM64_32:INT32_MIN_ (-2147483647 -1)
+// ARM64_32:UINT32_MAX_ 4294967295U
+// ARM64_32:INT_LEAST32_MIN_ (-2147483647 -1)
+// ARM64_32:INT_LEAST32_MAX_ 2147483647
+// ARM64_32:UINT_LEAST32_MAX_ 4294967295U
+// A

[PATCH] D61939: AArch64: add support for arm64_23 (ILP32) IR generation

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

Oops, yes. I'll leave it wrong though, the best that could come out of any 
attempt to change it would be to split the thread on llvm-commits.


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

https://reviews.llvm.org/D61939



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

[PATCH] D55586: Basic: make `int_least64_t` and `int_fast64_t` match on Darwin

[Tim Northover via Phabricator via cfe-commits]

t.p.northover accepted this revision.
t.p.northover added a comment.
This revision is now accepted and ready to land.

LGTM; well spotted!


Repository:
  rC Clang

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

https://reviews.llvm.org/D55586



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

[PATCH] D67436: CodeGen: set correct result for atomic compound expressions

[Tim Northover via Phabricator via cfe-commits]

t.p.northover created this revision.
Herald added subscribers: jfb, mcrosier.
Herald added a project: clang.

Atomic compound expressions try to use atomicrmw if possible, but this path 
doesn't set the `Result` variable, leaving it to crash in later code if 
anything ever tries to use the result of the expression. This fixes that issue 
by recalculating the new value based on the old one atomically loaded.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67436

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGen/atomic_ops.c

Index: clang/test/CodeGen/atomic_ops.c
===
--- clang/test/CodeGen/atomic_ops.c
+++ clang/test/CodeGen/atomic_ops.c
@@ -37,3 +37,56 @@
 // CHECK: {{store atomic|call void @__atomic_store}}
   x += y;
 }
+
+_Atomic(int) compound_add(_Atomic(int) in) {
+// CHECK-LABEL: @compound_add
+// CHECK: [[OLD:%.*]] = atomicrmw add i32* {{.*}}, i32 5 seq_cst
+// CHECK: [[NEW:%.*]] = add i32 [[OLD]], 5
+// CHECK: ret i32 [[NEW]]
+
+  return (in += 5);
+}
+
+_Atomic(int) compound_sub(_Atomic(int) in) {
+// CHECK-LABEL: @compound_sub
+// CHECK: [[OLD:%.*]] = atomicrmw sub i32* {{.*}}, i32 5 seq_cst
+// CHECK: [[NEW:%.*]] = sub i32 [[OLD]], 5
+// CHECK: ret i32 [[NEW]]
+
+  return (in -= 5);
+}
+
+_Atomic(int) compound_xor(_Atomic(int) in) {
+// CHECK-LABEL: @compound_xor
+// CHECK: [[OLD:%.*]] = atomicrmw xor i32* {{.*}}, i32 5 seq_cst
+// CHECK: [[NEW:%.*]] = xor i32 [[OLD]], 5
+// CHECK: ret i32 [[NEW]]
+
+  return (in ^= 5);
+}
+
+_Atomic(int) compound_or(_Atomic(int) in) {
+// CHECK-LABEL: @compound_or
+// CHECK: [[OLD:%.*]] = atomicrmw or i32* {{.*}}, i32 5 seq_cst
+// CHECK: [[NEW:%.*]] = or i32 [[OLD]], 5
+// CHECK: ret i32 [[NEW]]
+
+  return (in |= 5);
+}
+
+_Atomic(int) compound_and(_Atomic(int) in) {
+// CHECK-LABEL: @compound_and
+// CHECK: [[OLD:%.*]] = atomicrmw and i32* {{.*}}, i32 5 seq_cst
+// CHECK: [[NEW:%.*]] = and i32 [[OLD]], 5
+// CHECK: ret i32 [[NEW]]
+
+  return (in &= 5);
+}
+
+_Atomic(int) compound_mul(_Atomic(int) in) {
+// CHECK-LABEL: @compound_mul
+// CHECK: cmpxchg i32* {{%.*}}, i32 {{%.*}}, i32 [[NEW:%.*]] seq_cst seq_cst
+// CHECK: ret i32 [[NEW]]
+
+  return (in *= 5);
+}
Index: clang/lib/CodeGen/CGExprScalar.cpp
===
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -2840,7 +2840,8 @@
   CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow)) &&
 CGF.getLangOpts().getSignedOverflowBehavior() !=
 LangOptions::SOB_Trapping) {
-  llvm::AtomicRMWInst::BinOp aop = llvm::AtomicRMWInst::BAD_BINOP;
+  llvm::AtomicRMWInst::BinOp AtomicOp = llvm::AtomicRMWInst::BAD_BINOP;
+  llvm::Instruction::BinaryOps Op;
   switch (OpInfo.Opcode) {
 // We don't have atomicrmw operands for *, %, /, <<, >>
 case BO_MulAssign: case BO_DivAssign:
@@ -2849,30 +2850,40 @@
 case BO_ShrAssign:
   break;
 case BO_AddAssign:
-  aop = llvm::AtomicRMWInst::Add;
+  AtomicOp = llvm::AtomicRMWInst::Add;
+  Op = llvm::Instruction::Add;
   break;
 case BO_SubAssign:
-  aop = llvm::AtomicRMWInst::Sub;
+  AtomicOp = llvm::AtomicRMWInst::Sub;
+  Op = llvm::Instruction::Sub;
   break;
 case BO_AndAssign:
-  aop = llvm::AtomicRMWInst::And;
+  AtomicOp = llvm::AtomicRMWInst::And;
+  Op = llvm::Instruction::And;
   break;
 case BO_XorAssign:
-  aop = llvm::AtomicRMWInst::Xor;
+  AtomicOp = llvm::AtomicRMWInst::Xor;
+  Op = llvm::Instruction::Xor;
   break;
 case BO_OrAssign:
-  aop = llvm::AtomicRMWInst::Or;
+  AtomicOp = llvm::AtomicRMWInst::Or;
+  Op = llvm::Instruction::Or;
   break;
 default:
   llvm_unreachable("Invalid compound assignment type");
   }
-  if (aop != llvm::AtomicRMWInst::BAD_BINOP) {
-llvm::Value *amt = CGF.EmitToMemory(
+  if (AtomicOp != llvm::AtomicRMWInst::BAD_BINOP) {
+llvm::Value *Amt = CGF.EmitToMemory(
 EmitScalarConversion(OpInfo.RHS, E->getRHS()->getType(), LHSTy,
  E->getExprLoc()),
 LHSTy);
-Builder.CreateAtomicRMW(aop, LHSLV.getPointer(), amt,
+Value *OldVal = Builder.CreateAtomicRMW(
+AtomicOp, LHSLV.getPointer(), Amt,
 llvm::AtomicOrdering::SequentiallyConsistent);
+
+// Since operation is atomic, the result type is guaranteed to be the
+// same as the input in LLVM terms.
+Result = Builder.CreateBinOp(Op, OldVal, Amt);
 return LHSLV;
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67436: CodeGen: set correct result for atomic compound expressions

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67436



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

[PATCH] D45319: [Atomics] warn about misaligned atomic accesses using libcalls

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

Ping.


Repository:
  rC Clang

https://reviews.llvm.org/D45319



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

[PATCH] D45319: [Atomics] warn about misaligned atomic accesses using libcalls

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added inline comments.



Comment at: clang/lib/CodeGen/CGAtomic.cpp:883
   if (UseLibcall) {
+CGM.getDiags().Report(E->getLocStart(), diag::warn_atomic_op_misaligned);
+

compnerd wrote:
> It is kinda unfortunate that you need to look up 125 lines to get the context 
> that the call here is implied by a lack of alignment.  Perhaps we can rename 
> `UseLibcall` to `UnsuitableAligned` or something?
I'd be OK with that, or I could just move the diagnostic further up so it is 
next to the definition (or maybe just after to avoid the atomic_init case)?


Repository:
  rC Clang

https://reviews.llvm.org/D45319



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

[PATCH] D45319: [Atomics] warn about misaligned atomic accesses using libcalls

[Tim Northover via Phabricator via cfe-commits]

t.p.northover updated this revision to Diff 143506.
t.p.northover added a comment.

Moved diagnostic emission next to where the decision is made.


Repository:
  rC Clang

https://reviews.llvm.org/D45319

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGAtomic.cpp
  clang/test/CodeGen/atomics-sema-alignment.c


Index: clang/test/CodeGen/atomics-sema-alignment.c
===
--- /dev/null
+++ clang/test/CodeGen/atomics-sema-alignment.c
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -triple aarch64-linux-gnu %s -emit-llvm -o /dev/null -verify
+
+typedef struct {
+  int a, b;
+} IntPair;
+
+typedef struct {
+  long long a;
+} LongStruct;
+
+typedef int __attribute__((aligned(1))) unaligned_int;
+
+void func(IntPair *p) {
+  IntPair res;
+  __atomic_load(p, &res, 0); // expected-warning {{misaligned or large atomic 
operation may incur significant performance penalty}}
+  __atomic_store(p, &res, 0); // expected-warning {{misaligned or large atomic 
operation may incur significant performance penalty}}
+  __atomic_fetch_add((unaligned_int *)p, 1, 2); // expected-warning 
{{misaligned or large atomic operation may incur significant performance 
penalty}}
+  __atomic_fetch_sub((unaligned_int *)p, 1, 3); // expected-warning 
{{misaligned or large atomic operation may incur significant performance 
penalty}}
+}
+
+void func1(LongStruct *p) {
+  LongStruct res;
+  __atomic_load(p, &res, 0);
+  __atomic_store(p, &res, 0);
+  __atomic_fetch_add((int *)p, 1, 2);
+  __atomic_fetch_sub((int *)p, 1, 3);
+}
Index: clang/lib/CodeGen/CGAtomic.cpp
===
--- clang/lib/CodeGen/CGAtomic.cpp
+++ clang/lib/CodeGen/CGAtomic.cpp
@@ -18,6 +18,7 @@
 #include "TargetInfo.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/CodeGen/CGFunctionInfo.h"
+#include "clang/Sema/SemaDiagnostic.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Intrinsics.h"
@@ -751,19 +752,22 @@
   Address Dest = Address::invalid();
   Address Ptr = EmitPointerWithAlignment(E->getPtr());
 
+  if (E->getOp() == AtomicExpr::AO__c11_atomic_init ||
+  E->getOp() == AtomicExpr::AO__opencl_atomic_init) {
+LValue lvalue = MakeAddrLValue(Ptr, AtomicTy);
+EmitAtomicInit(E->getVal1(), lvalue);
+return RValue::get(nullptr);
+  }
+
   CharUnits sizeChars, alignChars;
   std::tie(sizeChars, alignChars) = getContext().getTypeInfoInChars(AtomicTy);
   uint64_t Size = sizeChars.getQuantity();
   unsigned MaxInlineWidthInBits = getTarget().getMaxAtomicInlineWidth();
   bool UseLibcall = ((Ptr.getAlignment() % sizeChars) != 0 ||
  getContext().toBits(sizeChars) > MaxInlineWidthInBits);
 
-  if (E->getOp() == AtomicExpr::AO__c11_atomic_init ||
-  E->getOp() == AtomicExpr::AO__opencl_atomic_init) {
-LValue lvalue = MakeAddrLValue(Ptr, AtomicTy);
-EmitAtomicInit(E->getVal1(), lvalue);
-return RValue::get(nullptr);
-  }
+  if (UseLibcall)
+CGM.getDiags().Report(E->getLocStart(), diag::warn_atomic_op_misaligned);
 
   llvm::Value *Order = EmitScalarExpr(E->getOrder());
   llvm::Value *Scope =
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7111,6 +7111,9 @@
   InGroup>;
 def err_atomic_op_has_invalid_synch_scope : Error<
   "synchronization scope argument to atomic operation is invalid">;
+def warn_atomic_op_misaligned : Warning<
+  "misaligned or large atomic operation may incur significant performance 
penalty">,
+  InGroup>;
 
 def err_overflow_builtin_must_be_int : Error<
   "operand argument to overflow builtin must be an integer (%0 invalid)">;


Index: clang/test/CodeGen/atomics-sema-alignment.c
===
--- /dev/null
+++ clang/test/CodeGen/atomics-sema-alignment.c
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -triple aarch64-linux-gnu %s -emit-llvm -o /dev/null -verify
+
+typedef struct {
+  int a, b;
+} IntPair;
+
+typedef struct {
+  long long a;
+} LongStruct;
+
+typedef int __attribute__((aligned(1))) unaligned_int;
+
+void func(IntPair *p) {
+  IntPair res;
+  __atomic_load(p, &res, 0); // expected-warning {{misaligned or large atomic operation may incur significant performance penalty}}
+  __atomic_store(p, &res, 0); // expected-warning {{misaligned or large atomic operation may incur significant performance penalty}}
+  __atomic_fetch_add((unaligned_int *)p, 1, 2); // expected-warning {{misaligned or large atomic operation may incur significant performance penalty}}
+  __atomic_fetch_sub((unaligned_int *)p, 1, 3); // expected-warning {{misaligned or large atomic operation may incur significant performance penalty}}
+}
+
+void func1(LongStruct *p) {
+  LongStruct res;
+  __atomic_load(p, &res, 0);
+  __atomic_s

[PATCH] D45319: [Atomics] warn about misaligned atomic accesses using libcalls

[Tim Northover via Phabricator via cfe-commits]

t.p.northover closed this revision.
t.p.northover added a comment.

Ah, I see you approved it before, presumably with the suggested changes. I've 
committed as r330566.


Repository:
  rC Clang

https://reviews.llvm.org/D45319



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

[PATCH] D63131: arm64_32: implement the desired ABI for the ILP32 triple.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63131



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

[PATCH] D63131: arm64_32: implement the desired ABI for the ILP32 triple.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover updated this revision to Diff 225259.
t.p.northover added a comment.

Updating diff. How target features are handled changed slightly upstream.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63131

Files:
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/arm64_32-vaarg.c
  clang/test/CodeGen/arm64_32.c
  clang/test/CodeGen/builtins-arm64.c
  clang/test/CodeGen/target-data.c
  clang/test/CodeGenCXX/armv7k.cpp
  clang/test/Driver/aarch64-cpus.c
  clang/test/Driver/arm64_32-link.c
  clang/test/Preprocessor/aarch64-target-features.c
  clang/test/Preprocessor/arm64_32.c
  clang/test/Preprocessor/init-v7k-compat.c
  clang/test/Preprocessor/stdint.c
  clang/test/Sema/aarch64-neon-vector-types.c
  clang/test/Sema/types.c

Index: clang/test/Sema/types.c
===
--- clang/test/Sema/types.c
+++ clang/test/Sema/types.c
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 %s -fblocks -pedantic -verify -triple=mips64-linux-gnu
 // RUN: %clang_cc1 %s -fblocks -pedantic -verify -triple=x86_64-unknown-linux
 // RUN: %clang_cc1 %s -fblocks -pedantic -verify -triple=x86_64-unknown-linux-gnux32
+// RUN: %clang_cc1 %s -fblocks -pedantic -pedantic -verify -triple=arm64_32-apple-ios7.0
 
 // rdar://6097662
 typedef int (*T)[2];
Index: clang/test/Sema/aarch64-neon-vector-types.c
===
--- clang/test/Sema/aarch64-neon-vector-types.c
+++ clang/test/Sema/aarch64-neon-vector-types.c
@@ -3,6 +3,8 @@
 // RUN: %clang_cc1 %s -triple arm64-none-linux-gnu -target-feature +neon -fsyntax-only -verify
 // RUN: %clang_cc1 %s -triple arm64-none-linux-gnu -target-feature +neon -DUSE_LONG -fsyntax-only -verify
 
+// RUN: %clang_cc1 %s -triple arm64_32-apple-ios -target-feature +neon -fsyntax-only -verify
+
 typedef float float32_t;
 typedef unsigned char poly8_t;
 typedef unsigned short poly16_t;
Index: clang/test/Preprocessor/stdint.c
===
--- clang/test/Preprocessor/stdint.c
+++ clang/test/Preprocessor/stdint.c
@@ -105,6 +105,113 @@
 // ARM:INTMAX_C_(0) 0LL
 // ARM:UINTMAX_C_(0) 0ULL
 //
+// RUN: %clang_cc1 -E -ffreestanding -triple=arm64_32-apple-ios7.0 %s | FileCheck -check-prefix ARM64_32 %s
+//
+// ARM64_32:typedef long long int int64_t;
+// ARM64_32:typedef long long unsigned int uint64_t;
+// ARM64_32:typedef int64_t int_least64_t;
+// ARM64_32:typedef uint64_t uint_least64_t;
+// ARM64_32:typedef int64_t int_fast64_t;
+// ARM64_32:typedef uint64_t uint_fast64_t;
+//
+// ARM64_32:typedef int int32_t;
+// ARM64_32:typedef unsigned int uint32_t;
+// ARM64_32:typedef int32_t int_least32_t;
+// ARM64_32:typedef uint32_t uint_least32_t;
+// ARM64_32:typedef int32_t int_fast32_t;
+// ARM64_32:typedef uint32_t uint_fast32_t;
+// 
+// ARM64_32:typedef short int16_t;
+// ARM64_32:typedef unsigned short uint16_t;
+// ARM64_32:typedef int16_t int_least16_t;
+// ARM64_32:typedef uint16_t uint_least16_t;
+// ARM64_32:typedef int16_t int_fast16_t;
+// ARM64_32:typedef uint16_t uint_fast16_t;
+//
+// ARM64_32:typedef signed char int8_t;
+// ARM64_32:typedef unsigned char uint8_t;
+// ARM64_32:typedef int8_t int_least8_t;
+// ARM64_32:typedef uint8_t uint_least8_t;
+// ARM64_32:typedef int8_t int_fast8_t;
+// ARM64_32:typedef uint8_t uint_fast8_t;
+//
+// ARM64_32:typedef long int intptr_t;
+// ARM64_32:typedef long unsigned int uintptr_t;
+// 
+// ARM64_32:typedef long long int intmax_t;
+// ARM64_32:typedef long long unsigned int uintmax_t;
+//
+// ARM64_32:INT8_MAX_ 127
+// ARM64_32:INT8_MIN_ (-127 -1)
+// ARM64_32:UINT8_MAX_ 255
+// ARM64_32:INT_LEAST8_MIN_ (-127 -1)
+// ARM64_32:INT_LEAST8_MAX_ 127
+// ARM64_32:UINT_LEAST8_MAX_ 255
+// ARM64_32:INT_FAST8_MIN_ (-127 -1)
+// ARM64_32:INT_FAST8_MAX_ 127
+// ARM64_32:UINT_FAST8_MAX_ 255
+//
+// ARM64_32:INT16_MAX_ 32767
+// ARM64_32:INT16_MIN_ (-32767 -1)
+// ARM64_32:UINT16_MAX_ 65535
+// ARM64_32:INT_LEAST16_MIN_ (-32767 -1)
+// ARM64_32:INT_LEAST16_MAX_ 32767
+// ARM64_32:UINT_LEAST16_MAX_ 65535
+// ARM64_32:INT_FAST16_MIN_ (-32767 -1)
+// ARM64_32:INT_FAST16_MAX_ 32767
+// ARM64_32:UINT_FAST16_MAX_ 65535
+//
+// ARM64_32:INT32_MAX_ 2147483647
+// ARM64_32:INT32_MIN_ (-2147483647 -1)
+// ARM64_32:UINT32_MAX_ 4294967295U
+// ARM64_32:INT_LEAST32_MIN_ (-2147483647 -1)
+// ARM64_32:INT_LEAST32_MAX_ 2147483647
+// ARM64_32:UINT_LEAST32_MAX_ 4294967295U
+// ARM64_32:INT_FAST32_MIN_ (-2147483647 -1)
+// ARM64_32:INT_FAST32_MAX_ 2147483647
+// ARM64_32:UINT_FAST32_MAX_ 4294967295U
+//

[PATCH] D80910: AArch64+ARM: make LLVM consider system registers volatile to prevent unsound optimizations.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover abandoned this revision.
t.p.northover added a comment.

Sorry. Keyboard decided to create diff before I'd filled everything in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80910



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

[PATCH] D80910: AArch64+ARM: make LLVM consider system registers volatile to prevent unsound optimizations.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover created this revision.
Herald added subscribers: llvm-commits, cfe-commits, danielkiss, jdoerfert, 
asbirlea, hiraditya, kristof.beyls, mcrosier.
Herald added projects: clang, LLVM.
t.p.northover abandoned this revision.
t.p.northover added a comment.

Sorry. Keyboard decided to create diff before I'd filled everything in.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80910

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-arm.c
  clang/test/CodeGen/builtins-arm64.c
  llvm/docs/LangRef.rst
  llvm/include/llvm/IR/Intrinsics.td
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/test/Transforms/LICM/read-volatile-register.ll

Index: llvm/test/Transforms/LICM/read-volatile-register.ll
===
--- /dev/null
+++ llvm/test/Transforms/LICM/read-volatile-register.ll
@@ -0,0 +1,30 @@
+; RUN: opt -S -licm %s | FileCheck %s
+
+; Volatile register shouldn't be hoisted ourside loops.
+define i32 @test_read() {
+; CHECK-LABEL: define i32 @test_read()
+; CHECK: br label %loop
+; CHECK: loop:
+; CHECK: %counter = tail call i64 @llvm.read_volatile_register
+
+entry:
+  br label %loop
+
+loop:
+  %i = phi i32 [ 0, %entry ], [ %i.next, %inc ]
+  %counter = tail call i64 @llvm.read_volatile_register.i64(metadata !1)
+  %tst = icmp ult i64 %counter, 1000
+  br i1 %tst, label %inc, label %done
+
+inc:
+  %i.next = add nuw nsw i32 %i, 1
+  br label %loop
+
+done:
+  ret i32 %i
+}
+
+declare i64 @llvm.read_register.i64(metadata)
+declare i64 @llvm.read_volatile_register.i64(metadata)
+
+!1 = !{!"cntpct_el0"}
Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -5665,6 +5665,7 @@
  TLI.getFrameIndexTy(DAG.getDataLayout()),
  getValue(I.getArgOperand(0;
 return;
+  case Intrinsic::read_volatile_register:
   case Intrinsic::read_register: {
 Value *Reg = I.getArgOperand(0);
 SDValue Chain = getRoot();
Index: llvm/include/llvm/IR/Intrinsics.td
===
--- llvm/include/llvm/IR/Intrinsics.td
+++ llvm/include/llvm/IR/Intrinsics.td
@@ -456,6 +456,9 @@
[IntrReadMem], "llvm.read_register">;
 def int_write_register : Intrinsic<[], [llvm_metadata_ty, llvm_anyint_ty],
[], "llvm.write_register">;
+def int_read_volatile_register  : Intrinsic<[llvm_anyint_ty], [llvm_metadata_ty],
+[IntrHasSideEffects],
+ "llvm.read_volatile_register">;
 
 // Gets the address of the local variable area. This is typically a copy of the
 // stack, frame, or base pointer depending on the type of prologue.
Index: llvm/docs/LangRef.rst
===
--- llvm/docs/LangRef.rst
+++ llvm/docs/LangRef.rst
@@ -11538,9 +11538,11 @@
 '``llvm.localrecover``'.
 
 .. _int_read_register:
+.. _int_read_volatile_register:
 .. _int_write_register:
 
-'``llvm.read_register``' and '``llvm.write_register``' Intrinsics
+'``llvm.read_register``', '``llvm.read_volatile_register``', and
+'``llvm.write_register``' Intrinsics
 ^
 
 Syntax:
@@ -11550,6 +11552,8 @@
 
   declare i32 @llvm.read_register.i32(metadata)
   declare i64 @llvm.read_register.i64(metadata)
+  declare i32 @llvm.read_volatile_register.i32(metadata)
+  declare i64 @llvm.read_volatile_register.i64(metadata)
   declare void @llvm.write_register.i32(metadata, i32 @value)
   declare void @llvm.write_register.i64(metadata, i64 @value)
   !0 = !{!"sp\00"}
@@ -11557,17 +11561,21 @@
 Overview:
 "
 
-The '``llvm.read_register``' and '``llvm.write_register``' intrinsics
-provides access to the named register. The register must be valid on
-the architecture being compiled to. The type needs to be compatible
-with the register being read.
+The '``llvm.read_register``', '``llvm.read_volatile_register``', and
+'``llvm.write_register``' intrinsics provide access to the named register.
+The register must be valid on the architecture being compiled to. The type
+needs to be compatible with the register being read.
 
 Semantics:
 ""
 
-The '``llvm.read_register``' intrinsic returns the current value of the
-register, where possible. The '``llvm.write_register``' intrinsic sets
-the current value of the register, where possible.
+The '``llvm.read_register``' and '``llvm.read_volatile_register``' intrinsics
+return the current value of the register, where possible. The
+'``llvm.write_register``' intrinsic sets the current value of the register,
+where possible.
+
+A call t

[PATCH] D80911: AArch64+ARM: make LLVM consider system registers volatile to prevent unsound optimizations.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover created this revision.
Herald added subscribers: danielkiss, jdoerfert, asbirlea, hiraditya, 
kristof.beyls, mcrosier.
Herald added projects: clang, LLVM.

Some of the system registers readable on AArch64 & ARM platforms return 
different values with each read (for example a timer counter), these shouldn't 
be hoisted outside loops or otherwise interfered with, but the normal 
@llvm.read_register intrinsic is only considered to read memory.

This introduces a separate @llvm.read_volatile_register intrinsic and maps all 
system-registers on ARM platforms to use it for the __builtin_arm_rsr calls. 
Registers declared with `asm("r9")` or similar are unaffected.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80911

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-arm.c
  clang/test/CodeGen/builtins-arm64.c
  llvm/docs/LangRef.rst
  llvm/include/llvm/IR/Intrinsics.td
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/test/Transforms/LICM/read-volatile-register.ll

Index: llvm/test/Transforms/LICM/read-volatile-register.ll
===
--- /dev/null
+++ llvm/test/Transforms/LICM/read-volatile-register.ll
@@ -0,0 +1,30 @@
+; RUN: opt -S -licm %s | FileCheck %s
+
+; Volatile register shouldn't be hoisted ourside loops.
+define i32 @test_read() {
+; CHECK-LABEL: define i32 @test_read()
+; CHECK: br label %loop
+; CHECK: loop:
+; CHECK: %counter = tail call i64 @llvm.read_volatile_register
+
+entry:
+  br label %loop
+
+loop:
+  %i = phi i32 [ 0, %entry ], [ %i.next, %inc ]
+  %counter = tail call i64 @llvm.read_volatile_register.i64(metadata !1)
+  %tst = icmp ult i64 %counter, 1000
+  br i1 %tst, label %inc, label %done
+
+inc:
+  %i.next = add nuw nsw i32 %i, 1
+  br label %loop
+
+done:
+  ret i32 %i
+}
+
+declare i64 @llvm.read_register.i64(metadata)
+declare i64 @llvm.read_volatile_register.i64(metadata)
+
+!1 = !{!"cntpct_el0"}
Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -5665,6 +5665,7 @@
  TLI.getFrameIndexTy(DAG.getDataLayout()),
  getValue(I.getArgOperand(0;
 return;
+  case Intrinsic::read_volatile_register:
   case Intrinsic::read_register: {
 Value *Reg = I.getArgOperand(0);
 SDValue Chain = getRoot();
Index: llvm/include/llvm/IR/Intrinsics.td
===
--- llvm/include/llvm/IR/Intrinsics.td
+++ llvm/include/llvm/IR/Intrinsics.td
@@ -456,6 +456,9 @@
[IntrReadMem], "llvm.read_register">;
 def int_write_register : Intrinsic<[], [llvm_metadata_ty, llvm_anyint_ty],
[], "llvm.write_register">;
+def int_read_volatile_register  : Intrinsic<[llvm_anyint_ty], [llvm_metadata_ty],
+[IntrHasSideEffects],
+ "llvm.read_volatile_register">;
 
 // Gets the address of the local variable area. This is typically a copy of the
 // stack, frame, or base pointer depending on the type of prologue.
Index: llvm/docs/LangRef.rst
===
--- llvm/docs/LangRef.rst
+++ llvm/docs/LangRef.rst
@@ -11538,9 +11538,11 @@
 '``llvm.localrecover``'.
 
 .. _int_read_register:
+.. _int_read_volatile_register:
 .. _int_write_register:
 
-'``llvm.read_register``' and '``llvm.write_register``' Intrinsics
+'``llvm.read_register``', '``llvm.read_volatile_register``', and
+'``llvm.write_register``' Intrinsics
 ^
 
 Syntax:
@@ -11550,6 +11552,8 @@
 
   declare i32 @llvm.read_register.i32(metadata)
   declare i64 @llvm.read_register.i64(metadata)
+  declare i32 @llvm.read_volatile_register.i32(metadata)
+  declare i64 @llvm.read_volatile_register.i64(metadata)
   declare void @llvm.write_register.i32(metadata, i32 @value)
   declare void @llvm.write_register.i64(metadata, i64 @value)
   !0 = !{!"sp\00"}
@@ -11557,17 +11561,21 @@
 Overview:
 "
 
-The '``llvm.read_register``' and '``llvm.write_register``' intrinsics
-provides access to the named register. The register must be valid on
-the architecture being compiled to. The type needs to be compatible
-with the register being read.
+The '``llvm.read_register``', '``llvm.read_volatile_register``', and
+'``llvm.write_register``' intrinsics provide access to the named register.
+The register must be valid on the architecture being compiled to. The type
+needs to be compatible with the register being read.
 
 Semantics:
 ""
 
-The '``llvm.read_register``' intrinsic returns the current value of the
-register, where possible. Th

[PATCH] D80911: AArch64+ARM: make LLVM consider system registers volatile to prevent unsound optimizations.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.
Herald added a reviewer: jdoerfert.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80911



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

[PATCH] D80911: AArch64+ARM: make LLVM consider system registers volatile to prevent unsound optimizations.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover closed this revision.
t.p.northover added a comment.

Thanks. Pushed to master as 5165b2b5fd5 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80911



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

[PATCH] D40948: Switch Clang's default C++ language target to C++14

[Tim Northover via Phabricator via cfe-commits]

t.p.northover created this revision.
Herald added a subscriber: mcrosier.

Hi all,

So, I've finally managed to run all the tests I wanted and get this out for 
review. Sorry it's taken so long. This patch switches Clang's default C++ 
target to C++14 across all platforms and updates the test-suite to pass.

I've managed to test Linux, macOS and iOS on both the regression tests and the 
test-suite (minus externals on Linux because it's my home machine). There's a 
separate patch for the test-suite which I'll attach to a follow-up message here.

I think most of the test changes are pretty straightforward and fall into a few 
broad categories:

- Changes to how the operands for "operator new" are promoted and otherwise 
wrangled in C++14. Slightly different CodeGen but essentially fine.
- Some tests explicitly testing C++ mode differences left the C++98 line 
without a -std=... argument, I added it.
- OpenMP CodeGen changes. As far as I can tell both are correct, but the tests 
are essentially unreadable so I gave up trying to adapt the output and just set 
it to C++98 mode.

I'm happy to rework anything people think needs changing.


Repository:
  rC Clang

https://reviews.llvm.org/D40948

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGenCXX/new-overflow.cpp
  clang/test/CodeGenCXX/new.cpp
  clang/test/CodeGenCXX/vtable-available-externally.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/Lexer/half-literal.cpp
  clang/test/OpenMP/taskloop_reduction_codegen.cpp
  clang/test/OpenMP/taskloop_simd_reduction_codegen.cpp
  clang/test/Parser/cxx1z-nested-namespace-definition.cpp
  clang/test/SemaCXX/new-array-size-conv.cpp
  clang/test/SemaCXX/new-delete.cpp
  clang/test/SemaCXX/unknown-type-name.cpp
  clang/test/SemaTemplate/class-template-decl.cpp
  clang/test/SemaTemplate/explicit-instantiation.cpp

Index: clang/test/SemaTemplate/explicit-instantiation.cpp
===
--- clang/test/SemaTemplate/explicit-instantiation.cpp
+++ clang/test/SemaTemplate/explicit-instantiation.cpp
@@ -124,10 +124,10 @@
 namespace undefined_static_data_member {
   template struct A {
 static int a; // expected-note {{here}}
-template static int b; // expected-note {{here}} expected-warning {{extension}}
+template static int b; // expected-note {{here}} expected-warning 0+ {{extension}}
   };
   struct B {
-template static int c; // expected-note {{here}} expected-warning {{extension}}
+template static int c; // expected-note {{here}} expected-warning 0+ {{extension}}
   };
 
   template int A::a; // expected-error {{explicit instantiation of undefined static data member 'a' of class template 'undefined_static_data_member::A'}}
@@ -137,14 +137,14 @@
 
   template struct C {
 static int a;
-template static int b; // expected-warning {{extension}}
+template static int b; // expected-warning 0+ {{extension}}
   };
   struct D {
-template static int c; // expected-warning {{extension}}
+template static int c; // expected-warning 0+ {{extension}}
   };
   template int C::a;
-  template template int C::b; // expected-warning {{extension}}
-  template int D::c; // expected-warning {{extension}}
+  template template int C::b; // expected-warning 0+ {{extension}}
+  template int D::c; // expected-warning 0+ {{extension}}
 
   template int C::a;
   template int C::b;
Index: clang/test/SemaTemplate/class-template-decl.cpp
===
--- clang/test/SemaTemplate/class-template-decl.cpp
+++ clang/test/SemaTemplate/class-template-decl.cpp
@@ -57,8 +57,7 @@
   template class X; // expected-error{{expression}}
 }
 
-template class X1 var; // expected-warning{{variable templates are a C++14 extension}} \
-   // expected-error {{variable has incomplete type 'class X1'}} \
+template class X1 var; // expected-error {{variable has incomplete type 'class X1'}} \
// expected-note {{forward declaration of 'X1'}}
 
 namespace M {
Index: clang/test/SemaCXX/unknown-type-name.cpp
===
--- clang/test/SemaCXX/unknown-type-name.cpp
+++ clang/test/SemaCXX/unknown-type-name.cpp
@@ -95,7 +95,10 @@
 template int h(T::type, int); // expected-error{{missing 'typename'}}
 template int h(T::type x, char); // expected-error{{missing 'typename'}}
 
-template int junk1(T::junk); // expected-warning{{variable templates are a C++14 extension}}
+template int junk1(T::junk);
+#if __cplusplus <= 201103L
+// expected-warning@-2 {{variable templates are a C++14 extension}}
+#endif
 template int junk2(T::junk) throw(); // expected-error{{missing 'typename'}}
 template int junk3(T::junk) = delete; // expected-error{{missing 'typename'}}
 #if __cplusplus <= 199711L
@@ -106,7 +109,11 @@
 
 // FIXME: We can tell this was intended to be a function because it does not
 //have a

[PATCH] D40948: Switch Clang's default C++ language target to C++14

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added inline comments.



Comment at: clang/test/SemaCXX/new-array-size-conv.cpp:1
-// RUN: %clang_cc1 -fsyntax-only -pedantic -verify %s
+// RUN: %clang_cc1 -fsyntax-only -pedantic -verify -std=gnu++98 %s
 // RUN: %clang_cc1 -fsyntax-only -pedantic -verify -std=c++98 %s

probinson wrote:
> I think the intent here was "whatever the current default is" and not 
> specifically gnu++98, because the next line explicitly specifies c++98.  So, 
> unless this test fails miserably for C++14 (which I wouldn't expect) I think 
> it should be adapted to whatever gets reported for that dialect.
The original commit of this file (r107229 from Doug Gregor) called out the fact 
that what's being tested is a GNU and C++0x extension.

I think that implies that the 3 run lines really *should* test gnu++98, c++98 
and c++11.


Repository:
  rC Clang

https://reviews.llvm.org/D40948



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

[PATCH] D40948: Switch Clang's default C++ language target to C++14

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

Thanks Richard. I'll file the bugs tomorrow for the issues you suggest. Do you 
see either of them blocking the change to C++14 as a default? On a scale of 
"no", "no but I want a commitment to fix them" and "yes" sort of thing.

Tonight I've just got one comment that may or may not be useful context before 
I give you a proper answer tomorrow:




Comment at: clang/test/CodeGenCXX/vtable-available-externally.cpp:1-2
-// RUN: %clang_cc1 %s -I%S -triple=x86_64-apple-darwin10 -emit-llvm -o %t
-// RUN: %clang_cc1 %s -I%S -triple=x86_64-apple-darwin10 -O2 
-disable-llvm-passes -emit-llvm -o %t.opt
+// RUN: %clang_cc1 %s -I%S -triple=x86_64-apple-darwin10 -std=c++98 -emit-llvm 
-o %t
+// RUN: %clang_cc1 %s -I%S -triple=x86_64-apple-darwin10 -std=c++98 -O2 
-disable-llvm-passes -emit-llvm -o %t.opt
 // RUN: FileCheck --check-prefix=CHECK-TEST1 %s < %t

rsmith wrote:
> Why does this one need a -std= flag?
It's a bit late here so I'll just give the proximal cause this evening in case 
that makes it obvious to you. I'll dig deeper tomorrow if not.

In this particular case (without the std flag) on the -O2 run line the "vtable 
for Test11::D" does not get marked "available_externally".

The diff on line 1 is definitely unneeded.



Comment at: clang/test/Parser/cxx1z-nested-namespace-definition.cpp:3-4
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++98
 // RUN: not %clang_cc1 -x c++ -fixit %t -Werror -DFIXIT
 // RUN: %clang_cc1 -x c++ %t -DFIXIT
 // RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++17 -Wc++14-compat

rsmith wrote:
> I think these two should also be `-std=c++98`, right?
Probably; I'll convince myself of that fact tomorrow.



Comment at: clang/test/SemaCXX/unknown-type-name.cpp:110-111
 
 // FIXME: We can tell this was intended to be a function because it does not
 //have a dependent nested name specifier.
+template int i(T::type, int());

rsmith wrote:
> Maybe delete this (incorrect in C++14) FIXME?
Sounds sensible.


Repository:
  rC Clang

https://reviews.llvm.org/D40948



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

[PATCH] D40948: Switch Clang's default C++ language target to C++14

[Tim Northover via Phabricator via cfe-commits]

t.p.northover updated this revision to Diff 126129.
t.p.northover added a comment.

Updating with tentative fixes to review comments.


Repository:
  rC Clang

https://reviews.llvm.org/D40948

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGenCXX/new-overflow.cpp
  clang/test/CodeGenCXX/new.cpp
  clang/test/CodeGenCXX/vtable-available-externally.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/Lexer/half-literal.cpp
  clang/test/OpenMP/taskloop_reduction_codegen.cpp
  clang/test/OpenMP/taskloop_simd_reduction_codegen.cpp
  clang/test/Parser/cxx1z-nested-namespace-definition.cpp
  clang/test/SemaCXX/new-array-size-conv.cpp
  clang/test/SemaCXX/new-delete.cpp
  clang/test/SemaCXX/unknown-type-name.cpp
  clang/test/SemaTemplate/class-template-decl.cpp
  clang/test/SemaTemplate/explicit-instantiation.cpp

Index: clang/test/SemaTemplate/explicit-instantiation.cpp
===
--- clang/test/SemaTemplate/explicit-instantiation.cpp
+++ clang/test/SemaTemplate/explicit-instantiation.cpp
@@ -124,10 +124,10 @@
 namespace undefined_static_data_member {
   template struct A {
 static int a; // expected-note {{here}}
-template static int b; // expected-note {{here}} expected-warning {{extension}}
+template static int b; // expected-note {{here}} expected-warning 0+ {{extension}}
   };
   struct B {
-template static int c; // expected-note {{here}} expected-warning {{extension}}
+template static int c; // expected-note {{here}} expected-warning 0+ {{extension}}
   };
 
   template int A::a; // expected-error {{explicit instantiation of undefined static data member 'a' of class template 'undefined_static_data_member::A'}}
@@ -137,14 +137,14 @@
 
   template struct C {
 static int a;
-template static int b; // expected-warning {{extension}}
+template static int b; // expected-warning 0+ {{extension}}
   };
   struct D {
-template static int c; // expected-warning {{extension}}
+template static int c; // expected-warning 0+ {{extension}}
   };
   template int C::a;
-  template template int C::b; // expected-warning {{extension}}
-  template int D::c; // expected-warning {{extension}}
+  template template int C::b; // expected-warning 0+ {{extension}}
+  template int D::c; // expected-warning 0+ {{extension}}
 
   template int C::a;
   template int C::b;
Index: clang/test/SemaTemplate/class-template-decl.cpp
===
--- clang/test/SemaTemplate/class-template-decl.cpp
+++ clang/test/SemaTemplate/class-template-decl.cpp
@@ -57,8 +57,7 @@
   template class X; // expected-error{{expression}}
 }
 
-template class X1 var; // expected-warning{{variable templates are a C++14 extension}} \
-   // expected-error {{variable has incomplete type 'class X1'}} \
+template class X1 var; // expected-error {{variable has incomplete type 'class X1'}} \
// expected-note {{forward declaration of 'X1'}}
 
 namespace M {
Index: clang/test/SemaCXX/unknown-type-name.cpp
===
--- clang/test/SemaCXX/unknown-type-name.cpp
+++ clang/test/SemaCXX/unknown-type-name.cpp
@@ -95,18 +95,23 @@
 template int h(T::type, int); // expected-error{{missing 'typename'}}
 template int h(T::type x, char); // expected-error{{missing 'typename'}}
 
-template int junk1(T::junk); // expected-warning{{variable templates are a C++14 extension}}
+template int junk1(T::junk);
+#if __cplusplus <= 201103L
+// expected-warning@-2 {{variable templates are a C++14 extension}}
+#endif
 template int junk2(T::junk) throw(); // expected-error{{missing 'typename'}}
 template int junk3(T::junk) = delete; // expected-error{{missing 'typename'}}
 #if __cplusplus <= 199711L
 //expected-warning@-2 {{deleted function definitions are a C++11 extension}}
 #endif
 
 template int junk4(T::junk j); // expected-error{{missing 'typename'}}
 
-// FIXME: We can tell this was intended to be a function because it does not
-//have a dependent nested name specifier.
-template int i(T::type, int()); // expected-warning{{variable templates are a C++14 extension}}
+template int i(T::type, int());
+#if __cplusplus <= 201103L
+// expected-warning@-2 {{variable templates are a C++14 extension}}
+#endif
+
 
 // FIXME: We know which type specifier should have been specified here. Provide
 //a fix-it to add 'typename A::type'
Index: clang/test/SemaCXX/new-delete.cpp
===
--- clang/test/SemaCXX/new-delete.cpp
+++ clang/test/SemaCXX/new-delete.cpp
@@ -80,27 +80,43 @@
   (void)new int[1.1];
 #if __cplusplus <= 199711L
   // expected-error@-2 {{array size expression must have integral or enumeration type, not 'double'}}
-#else
+#elif __cplusplus <= 201103L
   // expected-error@-4 {{array size expression must have integral or unscoped 

[PATCH] D40948: Switch Clang's default C++ language target to C++14

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added inline comments.



Comment at: clang/test/CodeGenCXX/new-overflow.cpp:88
   // CHECK:  [[N:%.*]] = sext i16 {{%.*}} to i32
-  // CHECK-NEXT: [[T0:%.*]] = icmp slt i32 [[N]], 0
-  // CHECK-NEXT: [[T1:%.*]] = select i1 [[T0]], i32 -1, i32 [[N]]
-  // CHECK-NEXT: call i8* @_Znaj(i32 [[T1]])
+  // CHECK-NEXT: call i8* @_Znaj(i32 [[N]])
   // CHECK:  getelementptr inbounds {{.*}}, i32 [[N]]

rsmith wrote:
> The changes in this file are a regression; C++14 requires us to check whether 
> the array bound prior to promotion is negative. Can you file a bug on that?
I've filed https://llvm.org/PR35573. Not quite sure what to do about this test 
until it's fixed though. Add a second RUN line to check both variants and then 
XFAIL it?



Comment at: clang/test/CodeGenCXX/vtable-available-externally.cpp:275
 struct C {
   virtual D& operator=(const D&);
 };

rsmith wrote:
> To make this test work in C++11 onwards, you need to add a virtual move 
> assignment operator here:
> 
> ```
> virtual D& operator=(D&&);
> ```
That didn't quite work. The issue appears to be that D has both of those 
implicitly defined in C++14 mode, but only the move assignment operator is used 
below. Speculative VTable emission requires all of them to be used.

So adding a "d = d;" line to the second g function fixes the test. Does that 
sound sane to you, or am I missing the point?



Comment at: clang/test/SemaCXX/unknown-type-name.cpp:98
 
-template int junk1(T::junk); // expected-warning{{variable 
templates are a C++14 extension}}
+template int junk1(T::junk);
+#if __cplusplus <= 201103L

rsmith wrote:
> Huh, we should probably have a `-Wvexing-parse` warning for this. Can you 
> file a bug?
I've filed https://llvm.org/PR35576. You may want to sanity check it though, I 
was pretty light on the detail because I wasn't sure of the exact diagnostic 
being proposed.


Repository:
  rC Clang

https://reviews.llvm.org/D40948



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

[PATCH] D40948: Switch Clang's default C++ language target to C++14

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1733
   // The PS4 uses C++11 as the default C++ standard.
-  if (T.isPS4())
-LangStd = LangStandard::lang_gnucxx11;
-  else
-LangStd = LangStandard::lang_gnucxx98;
+  LangStd = LangStandard::lang_gnucxx14;
   break;

filcab wrote:
> Why are you changing the PS4 default too?
Paul Robinson indicated that it was feasible back in March: 
http://lists.llvm.org/pipermail/cfe-dev/2017-March/052986.html. If that's 
changed I'd be happy to put it back to C++11, but he's one of the main people 
(rightly) bugging me about this so I'd be slightly surprised.

I also notice the comment crept back in. Bother.


Repository:
  rC Clang

https://reviews.llvm.org/D40948



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

[PATCH] D40948: Switch Clang's default C++ language target to C++14

[Tim Northover via Phabricator via cfe-commits]

t.p.northover closed this revision.
t.p.northover added a comment.

Thanks Richard, and all other reviewers. I committed this as r320250, with a 
couple of sanitizer test fixes as r320251 and r320284 (thanks Ahmed!).


Repository:
  rC Clang

https://reviews.llvm.org/D40948



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

[PATCH] D131153: AArch64: disable asynchronous unwind by default for MachO.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

> I don't think this is quite correct though? It'll turn off unwind tables for 
> AArch64 entirely, whereas we want to keep sync unwind tables.

You're right, sorry about that. Hopefully this refactoring with MaskRay's 
suggestion gets it right.




Comment at: clang/include/clang/Driver/ToolChain.h:501
+  /// IsAsyncUnwindTablesDefault - Does this tool chain use
+  /// -fasync-unwind-tables by default.
+  virtual bool

MaskRay wrote:
> smeenai wrote:
> > I believe the option is spelled `-fasynchronous-unwind-tables`.
> Changing `IsUnwindTablesDefault` to `unwindTablesDefaultLevel` may be 
> cleaner. 
I prefer it, though the Clang.cpp logic is unavoidably weird.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131153

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

[PATCH] D131153: AArch64: disable asynchronous unwind by default for MachO.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover updated this revision to Diff 458443.
Herald added subscribers: abrachet, mstorsjo, emaste.

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

https://reviews.llvm.org/D131153

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CrossWindows.cpp
  clang/lib/Driver/ToolChains/CrossWindows.h
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Driver/ToolChains/Darwin.h
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/FreeBSD.h
  clang/lib/Driver/ToolChains/Fuchsia.h
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Gnu.h
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/lib/Driver/ToolChains/MSVC.h
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/lib/Driver/ToolChains/MinGW.h
  clang/lib/Driver/ToolChains/NetBSD.h
  clang/lib/Driver/ToolChains/OpenBSD.cpp
  clang/lib/Driver/ToolChains/OpenBSD.h
  clang/test/Driver/clang-translation.c

Index: clang/test/Driver/clang-translation.c
===
--- clang/test/Driver/clang-translation.c
+++ clang/test/Driver/clang-translation.c
@@ -77,7 +77,11 @@
 
 // RUN: %clang -target arm64-apple-ios10 -### -S %s -arch arm64 2>&1 | \
 // RUN: FileCheck -check-prefix=ARM64-APPLE %s
-// ARM64-APPLE: -funwind-tables=2
+// ARM64-APPLE: -funwind-tables=1
+
+// RUN: %clang -target arm64-apple-ios10 -funwind-tables -### -S %s -arch arm64 2>&1 | \
+// RUN: FileCheck -check-prefix=ARM64-APPLE-UNWIND %s
+// ARM64-APPLE-UNWIND: -funwind-tables=1
 
 // RUN: %clang -target arm64-apple-ios10 -### -ffreestanding -S %s -arch arm64 2>&1 | \
 // RUN: FileCheck -check-prefix=ARM64-FREESTANDING-APPLE %s
Index: clang/lib/Driver/ToolChains/OpenBSD.h
===
--- clang/lib/Driver/ToolChains/OpenBSD.h
+++ clang/lib/Driver/ToolChains/OpenBSD.h
@@ -82,7 +82,8 @@
   std::string getCompilerRT(const llvm::opt::ArgList &Args, StringRef Component,
 FileType Type = ToolChain::FT_Static) const override;
 
-  bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args) const override;
+  UnwindTableLevel
+  getUnwindTableDefaultLevel(const llvm::opt::ArgList &Args) const override;
 
   LangOptions::StackProtectorMode
   GetDefaultStackProtectorLevel(bool KernelOrKext) const override {
Index: clang/lib/Driver/ToolChains/OpenBSD.cpp
===
--- clang/lib/Driver/ToolChains/OpenBSD.cpp
+++ clang/lib/Driver/ToolChains/OpenBSD.cpp
@@ -363,11 +363,12 @@
 
 bool OpenBSD::HasNativeLLVMSupport() const { return true; }
 
-bool OpenBSD::IsUnwindTablesDefault(const ArgList &Args) const {
-switch (getArch()) {
-  case llvm::Triple::arm:
-return false;
-  default:
-return true;
-}
+ToolChain::UnwindTableLevel
+OpenBSD::getUnwindTableDefaultLevel(const ArgList &Args) const {
+  switch (getArch()) {
+  case llvm::Triple::arm:
+return UTL_None;
+  default:
+return UTL_Asynchronous;
+  }
 }
Index: clang/lib/Driver/ToolChains/NetBSD.h
===
--- clang/lib/Driver/ToolChains/NetBSD.h
+++ clang/lib/Driver/ToolChains/NetBSD.h
@@ -65,8 +65,9 @@
   const llvm::opt::ArgList &DriverArgs,
   llvm::opt::ArgStringList &CC1Args) const override;
 
-  bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args) const override {
-return true;
+  UnwindTableLevel
+  getUnwindTableDefaultLevel(const llvm::opt::ArgList &Args) const override {
+return UTL_Asynchronous;
   }
 
   llvm::ExceptionHandling GetExceptionModel(
Index: clang/lib/Driver/ToolChains/MinGW.h
===
--- clang/lib/Driver/ToolChains/MinGW.h
+++ clang/lib/Driver/ToolChains/MinGW.h
@@ -66,7 +66,8 @@
   bool HasNativeLLVMSupport() const override;
 
   bool IsIntegratedAssemblerDefault() const override;
-  bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args) const override;
+  UnwindTableLevel
+  getUnwindTableDefaultLevel(const llvm::opt::ArgList &Args) const override;
   bool isPICDefault() const override;
   bool isPIEDefault(const llvm::opt::ArgList &Args) const override;
   bool isPICDefaultForced() const override;
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -482,15 +482,19 @@
   return NativeLLVMSupport;
 }
 
-bool toolchains::MinGW::IsUnwindTablesDefault(const ArgList &Args) const {
+ToolChain::UnwindTableLevel
+toolchains::MinGW::getUnwindTableDefaultLevel(const ArgList &Args) const {
   Arg *ExceptionArg = Args.getLastArg(options::OPT_fsjlj_exceptions,
   options::OPT_fseh_exceptions,
   options::OPT_fdwarf_ex

[PATCH] D131153: AArch64: disable asynchronous unwind by default for MachO.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover updated this revision to Diff 458444.
t.p.northover added a comment.
Herald added subscribers: bzcheeseman, sdasgup3, wenzhicui, wrengr, cota, 
teijeong, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, grosul1, 
Joonsoo, stephenneuendorffer, liufengdb, aartbik, mgester, arpith-jacob, 
nicolasvasilache, antiagainst, shauheen, rriddle, mehdi_amini.
Herald added a project: MLIR.

Quick rename of new method to flow better.


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

https://reviews.llvm.org/D131153

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CrossWindows.cpp
  clang/lib/Driver/ToolChains/CrossWindows.h
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Driver/ToolChains/Darwin.h
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/FreeBSD.h
  clang/lib/Driver/ToolChains/Fuchsia.h
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Gnu.h
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/lib/Driver/ToolChains/MSVC.h
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/lib/Driver/ToolChains/MinGW.h
  clang/lib/Driver/ToolChains/NetBSD.h
  clang/lib/Driver/ToolChains/OpenBSD.cpp
  clang/lib/Driver/ToolChains/OpenBSD.h
  clang/test/Driver/clang-translation.c
  mlir/lib/Dialect/Math/IR/MathOps.cpp

Index: mlir/lib/Dialect/Math/IR/MathOps.cpp
===
--- mlir/lib/Dialect/Math/IR/MathOps.cpp
+++ mlir/lib/Dialect/Math/IR/MathOps.cpp
@@ -420,17 +420,11 @@
 //===--===//
 
 OpFoldResult math::RoundEvenOp::fold(ArrayRef operands) {
-  return constFoldUnaryOpConditional(
-  operands, [](const APFloat &a) -> Optional {
-switch (a.getSizeInBits(a.getSemantics())) {
-case 64:
-  return APFloat(roundeven(a.convertToDouble()));
-case 32:
-  return APFloat(roundevenf(a.convertToFloat()));
-default:
-  return {};
-}
-  });
+  return constFoldUnaryOp(operands, [](const APFloat &a) {
+APFloat result(a);
+result.roundToIntegral(llvm::RoundingMode::NearestTiesToEven);
+return result;
+  });
 }
 
 /// Materialize an integer or floating point constant.
Index: clang/test/Driver/clang-translation.c
===
--- clang/test/Driver/clang-translation.c
+++ clang/test/Driver/clang-translation.c
@@ -77,7 +77,11 @@
 
 // RUN: %clang -target arm64-apple-ios10 -### -S %s -arch arm64 2>&1 | \
 // RUN: FileCheck -check-prefix=ARM64-APPLE %s
-// ARM64-APPLE: -funwind-tables=2
+// ARM64-APPLE: -funwind-tables=1
+
+// RUN: %clang -target arm64-apple-ios10 -funwind-tables -### -S %s -arch arm64 2>&1 | \
+// RUN: FileCheck -check-prefix=ARM64-APPLE-UNWIND %s
+// ARM64-APPLE-UNWIND: -funwind-tables=1
 
 // RUN: %clang -target arm64-apple-ios10 -### -ffreestanding -S %s -arch arm64 2>&1 | \
 // RUN: FileCheck -check-prefix=ARM64-FREESTANDING-APPLE %s
Index: clang/lib/Driver/ToolChains/OpenBSD.h
===
--- clang/lib/Driver/ToolChains/OpenBSD.h
+++ clang/lib/Driver/ToolChains/OpenBSD.h
@@ -82,7 +82,8 @@
   std::string getCompilerRT(const llvm::opt::ArgList &Args, StringRef Component,
 FileType Type = ToolChain::FT_Static) const override;
 
-  bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args) const override;
+  UnwindTableLevel
+  getUnwindTableDefaultLevel(const llvm::opt::ArgList &Args) const override;
 
   LangOptions::StackProtectorMode
   GetDefaultStackProtectorLevel(bool KernelOrKext) const override {
Index: clang/lib/Driver/ToolChains/OpenBSD.cpp
===
--- clang/lib/Driver/ToolChains/OpenBSD.cpp
+++ clang/lib/Driver/ToolChains/OpenBSD.cpp
@@ -363,11 +363,12 @@
 
 bool OpenBSD::HasNativeLLVMSupport() const { return true; }
 
-bool OpenBSD::IsUnwindTablesDefault(const ArgList &Args) const {
-switch (getArch()) {
-  case llvm::Triple::arm:
-return false;
-  default:
-return true;
-}
+ToolChain::UnwindTableLevel
+OpenBSD::getUnwindTableDefaultLevel(const ArgList &Args) const {
+  switch (getArch()) {
+  case llvm::Triple::arm:
+return UTL_None;
+  default:
+return UTL_Asynchronous;
+  }
 }
Index: clang/lib/Driver/ToolChains/NetBSD.h
===
--- clang/lib/Driver/ToolChains/NetBSD.h
+++ clang/lib/Driver/ToolChains/NetBSD.h
@@ -65,8 +65,9 @@
   const llvm::opt::ArgList &DriverArgs,
   llvm::opt::ArgStringList &CC1Args) const override;
 
-  bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args) const override {
-return true;
+  UnwindTableLevel
+  getUnwindTableDefaultLevel(const llvm::opt::ArgList &Args) const override {
+return UTL_Asynchr

[PATCH] D131153: AArch64: disable asynchronous unwind by default for MachO.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover updated this revision to Diff 459004.
t.p.northover marked an inline comment as done.
t.p.northover added a comment.

Switched to `enum class`.

> You may want to split the patch, with refactoring as the first, and the 
> Mach-O specific change as the second one.

I've got it split into two commits locally now. It's pretty much as you'd 
expect: first one NFC with no test changes, the second adds the test change and 
changes the return statement in `MachO::getDefaultUnwindTableLevel` from an 
unconditional `Asynchronous`.


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

https://reviews.llvm.org/D131153

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CrossWindows.cpp
  clang/lib/Driver/ToolChains/CrossWindows.h
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Driver/ToolChains/Darwin.h
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/FreeBSD.h
  clang/lib/Driver/ToolChains/Fuchsia.h
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Gnu.h
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/lib/Driver/ToolChains/MSVC.h
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/lib/Driver/ToolChains/MinGW.h
  clang/lib/Driver/ToolChains/NetBSD.h
  clang/lib/Driver/ToolChains/OpenBSD.cpp
  clang/lib/Driver/ToolChains/OpenBSD.h
  clang/test/Driver/clang-translation.c

Index: clang/test/Driver/clang-translation.c
===
--- clang/test/Driver/clang-translation.c
+++ clang/test/Driver/clang-translation.c
@@ -77,7 +77,11 @@
 
 // RUN: %clang -target arm64-apple-ios10 -### -S %s -arch arm64 2>&1 | \
 // RUN: FileCheck -check-prefix=ARM64-APPLE %s
-// ARM64-APPLE: -funwind-tables=2
+// ARM64-APPLE: -funwind-tables=1
+
+// RUN: %clang -target arm64-apple-ios10 -funwind-tables -### -S %s -arch arm64 2>&1 | \
+// RUN: FileCheck -check-prefix=ARM64-APPLE-UNWIND %s
+// ARM64-APPLE-UNWIND: -funwind-tables=1
 
 // RUN: %clang -target arm64-apple-ios10 -### -ffreestanding -S %s -arch arm64 2>&1 | \
 // RUN: FileCheck -check-prefix=ARM64-FREESTANDING-APPLE %s
Index: clang/lib/Driver/ToolChains/OpenBSD.h
===
--- clang/lib/Driver/ToolChains/OpenBSD.h
+++ clang/lib/Driver/ToolChains/OpenBSD.h
@@ -82,7 +82,8 @@
   std::string getCompilerRT(const llvm::opt::ArgList &Args, StringRef Component,
 FileType Type = ToolChain::FT_Static) const override;
 
-  bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args) const override;
+  UnwindTableLevel
+  getDefaultUnwindTableLevel(const llvm::opt::ArgList &Args) const override;
 
   LangOptions::StackProtectorMode
   GetDefaultStackProtectorLevel(bool KernelOrKext) const override {
Index: clang/lib/Driver/ToolChains/OpenBSD.cpp
===
--- clang/lib/Driver/ToolChains/OpenBSD.cpp
+++ clang/lib/Driver/ToolChains/OpenBSD.cpp
@@ -363,11 +363,12 @@
 
 bool OpenBSD::HasNativeLLVMSupport() const { return true; }
 
-bool OpenBSD::IsUnwindTablesDefault(const ArgList &Args) const {
-switch (getArch()) {
-  case llvm::Triple::arm:
-return false;
-  default:
-return true;
-}
+ToolChain::UnwindTableLevel
+OpenBSD::getDefaultUnwindTableLevel(const ArgList &Args) const {
+  switch (getArch()) {
+  case llvm::Triple::arm:
+return UnwindTableLevel::None;
+  default:
+return UnwindTableLevel::Asynchronous;
+  }
 }
Index: clang/lib/Driver/ToolChains/NetBSD.h
===
--- clang/lib/Driver/ToolChains/NetBSD.h
+++ clang/lib/Driver/ToolChains/NetBSD.h
@@ -65,8 +65,9 @@
   const llvm::opt::ArgList &DriverArgs,
   llvm::opt::ArgStringList &CC1Args) const override;
 
-  bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args) const override {
-return true;
+  UnwindTableLevel
+  getDefaultUnwindTableLevel(const llvm::opt::ArgList &Args) const override {
+return UnwindTableLevel::Asynchronous;
   }
 
   llvm::ExceptionHandling GetExceptionModel(
Index: clang/lib/Driver/ToolChains/MinGW.h
===
--- clang/lib/Driver/ToolChains/MinGW.h
+++ clang/lib/Driver/ToolChains/MinGW.h
@@ -66,7 +66,8 @@
   bool HasNativeLLVMSupport() const override;
 
   bool IsIntegratedAssemblerDefault() const override;
-  bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args) const override;
+  UnwindTableLevel
+  getDefaultUnwindTableLevel(const llvm::opt::ArgList &Args) const override;
   bool isPICDefault() const override;
   bool isPIEDefault(const llvm::opt::ArgList &Args) const override;
   bool isPICDefaultForced() const override;
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp

[PATCH] D131153: AArch64: disable asynchronous unwind by default for MachO.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover created this revision.
Herald added subscribers: kristof.beyls, mcrosier.
Herald added a project: All.
t.p.northover requested review of this revision.
Herald added a subscriber: MaskRay.
Herald added a project: clang.

AArch64 MachO has a compact unwind format where most functions' unwind info can 
be represented in just 4 bytes. But this cannot represent any asynchronous CFI 
function, so it's essentially disabled when that's used. This is a large 
code-size hit that we'd rather not take unless explicitly requested.

So this patch adds a new callback to switch the default on ARM64 MachO to 
synchronous. Command-line options can still turn it on if anyone wants to take 
the hit.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131153

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Driver/ToolChains/Darwin.h
  clang/test/Driver/clang-translation.c
  clang/test/Preprocessor/unwind-tables.c

Index: clang/test/Preprocessor/unwind-tables.c
===
--- clang/test/Preprocessor/unwind-tables.c
+++ clang/test/Preprocessor/unwind-tables.c
@@ -3,7 +3,7 @@
 
 // RUN: %clang %s -dM -E -target x86_64 | FileCheck %s
 // RUN: %clang %s -dM -E -target x86_64 -funwind-tables -fno-asynchronous-unwind-tables -g | FileCheck %s
-// RUN: %clang %s -dM -E -target aarch64-apple-darwin | FileCheck %s
+// RUN: %clang %s -dM -E -target aarch64-apple-darwin | FileCheck %s --check-prefix=NO
 // RUN: %clang %s -dM -E -target x86_64 -fno-asynchronous-unwind-tables -g | FileCheck %s
 // RUN: %clang %s -dM -E -target x86_64 -fno-asynchronous-unwind-tables -fexceptions | FileCheck %s
 
Index: clang/test/Driver/clang-translation.c
===
--- clang/test/Driver/clang-translation.c
+++ clang/test/Driver/clang-translation.c
@@ -78,7 +78,11 @@
 
 // RUN: %clang -target arm64-apple-ios10 -### -S %s -arch arm64 2>&1 | \
 // RUN: FileCheck -check-prefix=ARM64-APPLE %s
-// ARM64-APPLE: -funwind-tables=2
+// ARM64-APPLE-NOT: -funwind-tables
+
+// RUN: %clang -target arm64-apple-ios10 -funwind-tables -### -S %s -arch arm64 2>&1 | \
+// RUN: FileCheck -check-prefix=ARM64-APPLE-UNWIND %s
+// ARM64-APPLE-UNWIND: -funwind-tables=1
 
 // RUN: %clang -target arm64-apple-ios10 -### -ffreestanding -S %s -arch arm64 2>&1 | \
 // RUN: FileCheck -check-prefix=ARM64-FREESTANDING-APPLE %s
Index: clang/lib/Driver/ToolChains/Darwin.h
===
--- clang/lib/Driver/ToolChains/Darwin.h
+++ clang/lib/Driver/ToolChains/Darwin.h
@@ -256,6 +256,9 @@
 
   bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args) const override;
 
+  bool
+  IsAsyncUnwindTablesDefault(const llvm::opt::ArgList &Args) const override;
+
   RuntimeLibType GetDefaultRuntimeLibType() const override {
 return ToolChain::RLT_CompilerRT;
   }
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -2894,6 +2894,15 @@
true));
 }
 
+bool MachO::IsAsyncUnwindTablesDefault(const ArgList &Args) const {
+  // AArch64 has compact unwind format, making the size overhead from
+  // asynchronous unwind particularly bad, so disable by default.
+  if (getArch() == llvm::Triple::aarch64)
+return false;
+
+  return IsUnwindTablesDefault(Args);
+}
+
 bool MachO::UseDwarfDebugFlags() const {
   if (const char *S = ::getenv("RC_DEBUG_OPTIONS"))
 return S[0] != '\0';
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5387,11 +5387,12 @@
   // -fasynchronous-unwind-tables and -fnon-call-exceptions interact in more
   // complicated ways.
   auto SanitizeArgs = TC.getSanitizerArgs(Args);
-  bool AsyncUnwindTables = Args.hasFlag(
-  options::OPT_fasynchronous_unwind_tables,
-  options::OPT_fno_asynchronous_unwind_tables,
-  (TC.IsUnwindTablesDefault(Args) || SanitizeArgs.needsUnwindTables()) &&
-  !Freestanding);
+  bool AsyncUnwindTables =
+  Args.hasFlag(options::OPT_fasynchronous_unwind_tables,
+   options::OPT_fno_asynchronous_unwind_tables,
+   (TC.IsAsyncUnwindTablesDefault(Args) ||
+SanitizeArgs.needsUnwindTables()) &&
+   !Freestanding);
   bool UnwindTables = Args.hasFlag(options::OPT_funwind_tables,
options::OPT_fno_unwind_tables, false);
   if (AsyncUnwindTables)
Index: clang/include/clang/Driver/ToolChain.h
===
--- clang/include/clang/Driver/ToolChain.h
+++ clang/include/clang/Driver/ToolC

[PATCH] D119788: [AArch64] Add support for -march=native for Apple M1 CPU

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

Things have moved on since the ARM and (especially) PPC variants of that 
function were written. That field (despite the name) is now more of an ABI tag 
and not going to be updated with each CPU.

I think the modern replacement for it is `hw.cpufamily` obtained from `sysctl` 
(command line or `man 3 sysctl` for programmatically). The potential values 
(`CPUFAMILY_ARM_*` starting with Cyclone) are listed in  
https://opensource.apple.com/source/xnu/xnu-4570.41.2/osfmk/mach/machine.h.auto.html
 (which should be in the SDK under `mach/machine.h`). It warns against 
inferring features from this, which is exactly what we'd be doing, but still 
probably has a better chance of being right long term.

I'd probably default to the latest known one rather than earliest too, since an 
unknown family is more likely to be new than old.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119788

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

[PATCH] D119788: [AArch64] Add support for -march=native for Apple M1 CPU

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

Sorry, that was an old xnu version, the newest one is 
https://opensource.apple.com/source/xnu/xnu-7195.81.3/osfmk/mach/machine.h.auto.html
 which has

| `CPUFAMILY_ARM_CYCLONE`| 0x37a09642 | `apple-a7`  
 |
| `CPUFAMILY_ARM_TYPHOON`| 0x2c91a47e | `apple-a8`  
 |
| `CPUFAMILY_ARM_TWISTER`| 0x92fb37c8 | `apple-a9`  
 |
| `CPUFAMILY_ARM_HURRICANE`  | 0x67ceee93 | `apple-a10` 
 |
| `CPUFAMILY_ARM_MONSOON_MISTRAL`| 0xe81e7ef6 | `apple-a11` 
 |
| `CPUFAMILY_ARM_VORTEX_TEMPEST` | 0x07d34b9f | `apple-a12` 
 |
| `CPUFAMILY_ARM_LIGHTNING_THUNDER`  | 0x462504d2 | `apple-a13` 
 |
| `CPUFAMILY_ARM_FIRESTORM_ICESTORM` | 0x1b588bb3 | `apple-m1` (and 
`apple-a14`) |
|

We'll probably have to hard-code most of those because we need LLVM to be 
buildable on much older versions of Xcode that don't know about newer CPUs 
(likely Xcode 9.3 soon, but now older even than that).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119788

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

[PATCH] D34873: Fix miscompiled 32bit binaries by mingw

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added inline comments.



Comment at: lib/AST/ExprConstant.cpp:583
+uint64_t& GetArrayInitIndex() {
+return reinterpret_cast(ArrayInitIndex[0]);
+}

rnk wrote:
> Surely this will fault on SPARC or ARM or other ISAs that care about 
> alignment?
As well as being horribly undefined behaviour.


https://reviews.llvm.org/D34873



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

[PATCH] D35817: Ban implicit _Complex to scalar conversions in C++

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

Ping.


https://reviews.llvm.org/D35817



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

[PATCH] D35817: Ban implicit _Complex to scalar conversions in C++

[Tim Northover via Phabricator via cfe-commits]

t.p.northover updated this revision to Diff 109149.
t.p.northover added a comment.

Sounds like an improvement, I've updated the diff.


https://reviews.llvm.org/D35817

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CodeGenCXX/stmtexpr.cpp
  clang/test/OpenMP/atomic_capture_codegen.cpp
  clang/test/OpenMP/atomic_update_codegen.cpp
  clang/test/SemaCXX/complex-conversion.cpp
  clang/test/SemaCXX/complex-overload.cpp
  clang/test/SemaCXX/integer-overflow.cpp
  clang/test/SemaCXX/warn-absolute-value.cpp

Index: clang/test/SemaCXX/warn-absolute-value.cpp
===
--- clang/test/SemaCXX/warn-absolute-value.cpp
+++ clang/test/SemaCXX/warn-absolute-value.cpp
@@ -448,126 +448,23 @@
 }
 
 void test_complex_float(_Complex float x) {
-  (void)abs(x);
-  // expected-warning@-1 {{using integer absolute value function 'abs' when argument is of complex type}}
-  // expected-note@-2 {{use function 'cabsf' instead}}
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:9-[[@LINE-3]]:12}:"cabsf"
-  (void)labs(x);
-  // expected-warning@-1 {{using integer absolute value function 'labs' when argument is of complex type}}
-  // expected-note@-2 {{use function 'cabsf' instead}}
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:9-[[@LINE-3]]:13}:"cabsf"
-  (void)llabs(x);
-  // expected-warning@-1 {{using integer absolute value function 'llabs' when argument is of complex type}}
-  // expected-note@-2 {{use function 'cabsf' instead}}
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:9-[[@LINE-3]]:14}:"cabsf"
-
-  (void)fabsf(x);
-  // expected-warning@-1 {{using floating point absolute value function 'fabsf' when argument is of complex type}}
-  // expected-note@-2 {{use function 'cabsf' instead}}
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:9-[[@LINE-3]]:14}:"cabsf"
-  (void)fabs(x);
-  // expected-warning@-1 {{using floating point absolute value function 'fabs' when argument is of complex type}}
-  // expected-note@-2 {{use function 'cabsf' instead}}
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:9-[[@LINE-3]]:13}:"cabsf"
-  (void)fabsl(x);
-  // expected-warning@-1 {{using floating point absolute value function 'fabsl' when argument is of complex type}}
-  // expected-note@-2 {{use function 'cabsf' instead}}
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:9-[[@LINE-3]]:14}:"cabsf"
-
   (void)cabsf(x);
   (void)cabs(x);
   (void)cabsl(x);
 
-  (void)__builtin_abs(x);
-  // expected-warning@-1 {{using integer absolute value function '__builtin_abs' when argument is of complex type}}
-  // expected-note@-2 {{use function '__builtin_cabsf' instead}}
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:9-[[@LINE-3]]:22}:"__builtin_cabsf"
-  (void)__builtin_labs(x);
-  // expected-warning@-1 {{using integer absolute value function '__builtin_labs' when argument is of complex type}}
-  // expected-note@-2 {{use function '__builtin_cabsf' instead}}
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:9-[[@LINE-3]]:23}:"__builtin_cabsf"
-  (void)__builtin_llabs(x);
-  // expected-warning@-1 {{using integer absolute value function '__builtin_llabs' when argument is of complex type}}
-  // expected-note@-2 {{use function '__builtin_cabsf' instead}}
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:9-[[@LINE-3]]:24}:"__builtin_cabsf"
-
-  (void)__builtin_fabsf(x);
-  // expected-warning@-1 {{using floating point absolute value function '__builtin_fabsf' when argument is of complex type}}
-  // expected-note@-2 {{use function '__builtin_cabsf' instead}}
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:9-[[@LINE-3]]:24}:"__builtin_cabsf"
-  (void)__builtin_fabs(x);
-  // expected-warning@-1 {{using floating point absolute value function '__builtin_fabs' when argument is of complex type}}
-  // expected-note@-2 {{use function '__builtin_cabsf' instead}}
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:9-[[@LINE-3]]:23}:"__builtin_cabsf"
-  (void)__builtin_fabsl(x);
-  // expected-warning@-1 {{using floating point absolute value function '__builtin_fabsl' when argument is of complex type}}
-  // expected-note@-2 {{use function '__builtin_cabsf' instead}}
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:9-[[@LINE-3]]:24}:"__builtin_cabsf"
-
   (void)__builtin_cabsf(x);
   (void)__builtin_cabs(x);
   (void)__builtin_cabsl(x);
 }
 
 void test_complex_double(_Complex double x) {
-  (void)abs(x);
-  // expected-warning@-1 {{using integer absolute value function 'abs' when argument is of complex type}}
-  // expected-note@-2 {{use function 'cabs' instead}}
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:9-[[@LINE-3]]:12}:"cabs"
-  (void)labs(x);
-  // expected-warning@-1 {{using integer absolute value function 'labs' when argument is of complex type}}
-  // expected-note@-2 {{use function 'cabs' instead}}
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:9-[[@LINE-3]]:13}:"cabs"
-  (void)llabs(x);
-  // expected-warning@-1 {{using integer absolute value function 'llabs' when argument is of complex type}}
- 

[PATCH] D36272: [CodeGen][x86_64] Enable 'force_align_arg_pointer' attribute at x86_64

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

I've added "cfe-commits" as a subscriber since that's how we make sure the 
review request gets to the mailing list (which is still where review 
canonically takes place).


Repository:
  rL LLVM

https://reviews.llvm.org/D36272



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

[PATCH] D35817: Ban implicit _Complex to scalar conversions in C++

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

Pingy.


https://reviews.llvm.org/D35817



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

[PATCH] D35817: Ban implicit _Complex to scalar conversions in C++

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

> What's going on with the OpenMP test changes?

Compound assignment operators like "real /= complex" become illegal under the 
new rules (in C++) because somewhere there has to be an implicit conversion. I 
was pretty relieved to discover GCC also rejects the syntax because my change 
actually messes up the AST quite comprehensively if we wanted it to be legal 
(OpenMP does some weird stuff building its AST).


https://reviews.llvm.org/D35817



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

[PATCH] D70779: AArch64: add support for newer Apple CPUs

[Tim Northover via Phabricator via cfe-commits]

t.p.northover closed this revision.
t.p.northover added a comment.

Thanks Ahmed, pushed:

To github.com:llvm/llvm-project.git

  0a4daff6e26f..903e5c3028d6  master -> master


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

https://reviews.llvm.org/D70779



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

[PATCH] D87095: [Triple][MachO] Define "arm64e", an AArch64 subarch for Pointer Auth.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

I had a look through this and everything seemed to be in place. I think the ABI 
versioning is probably sufficient to prevent surprises and land this now, to 
avoid churn in the rest of the code. If more is really needed, we could add a 
temporary warning ("incomplete ABI") to the Clang driver.


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

https://reviews.llvm.org/D87095

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

[PATCH] D89959: UBSAN: emit distinctive traps in trapping mode

[Tim Northover via Phabricator via cfe-commits]

t.p.northover closed this revision.
t.p.northover added a comment.

Committed:

To github.com:llvm/llvm-project.git

  c54d827fdb12..c5978f42ec8e  main -> main


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

https://reviews.llvm.org/D89959

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

[PATCH] D92930: [Clang] Add vcmla and rotated variants for Arm ACLE.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

Looks fine to me except for one weird quirk that I don't think should be there.




Comment at: clang/lib/CodeGen/CGBuiltin.cpp:5237
   NEONMAP1(vcaddq_rot90_v, arm_neon_vcadd_rot90, Add1ArgType),
+  NEONMAP1(vcaddq_rot90_v, arm_neon_vcadd_rot90, Add1ArgType),
   NEONMAP1(vcage_v, arm_neon_vacge, 0),

This line looks identical to the one above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92930

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

[PATCH] D92930: [Clang] Add vcmla and rotated variants for Arm ACLE.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover accepted this revision.
t.p.northover added a comment.
This revision is now accepted and ready to land.

That's a nice improvement. Still LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92930

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

[PATCH] D89959: UBSAN: emit distinctive traps in trapping mode

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

With current CodeGen by the time you reach the trap you have no idea what came 
before so I think you'd still need a separate trap instruction per failure 
kind. So the Clang and generic LLVM side would be unaffected.

I suppose on X86 it could save a few bytes in .text and so maybe icache (though 
again we're talking about roughly 50% bloat for enabling UBSAN at all so this 
is kind of lost in the noise). On the whole I'm not a fan of inventing a new 
entirely separate scheme for that.


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

https://reviews.llvm.org/D89959

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

[PATCH] D89959: UBSAN: emit distinctive traps in trapping mode

[Tim Northover via Phabricator via cfe-commits]

t.p.northover marked an inline comment as done.
t.p.northover added a comment.

> Was this measured with all of -fsanitize=undefined enabled? If so, the actual 
> size overhead is likely lower, as only a subset of these checks get enabled 
> in production settings.

Yes, I used `-fsanitize=undefined -fsanitize-trap=all`. Fixed the LangRef but I 
won't upload another diff unless something more substantial changes.


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

https://reviews.llvm.org/D89959

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

[PATCH] D89959: UBSAN: emit distinctive traps in trapping mode

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added inline comments.



Comment at: llvm/include/llvm/IR/Intrinsics.td:1242
+def int_ubsantrap : Intrinsic<[], [llvm_i8_ty],
+  [IntrNoReturn, IntrCold, ImmArg>]>;
 

jdoerfert wrote:
> should this be readonly and inaccesiblememonly?
For `readonly` I'd say no (LangRef says "if a readonly function [...] has other 
side-effects, the behavior is undefined"), `inaccessiblememonly` maybe, but I 
doubt the definitions were written with something like a trap in mind so the 
whole topic seems pretty fuzzy to me.


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

https://reviews.llvm.org/D89959

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

[PATCH] D91147: AArch64: classify Triple::aarch64_32 as AArch64

[Tim Northover via Phabricator via cfe-commits]

t.p.northover created this revision.
Herald added subscribers: dexonsmith, danielkiss, s.egerton, simoncook, 
hiraditya, kristof.beyls, mcrosier.
Herald added a project: LLVM.
t.p.northover requested review of this revision.

We want `arm64_32` to omit leaf frame pointers. At the moment this is 
predicated on `isAArch64` and fails because `Triple::aarch64_32` doesn't count. 
That's logically incorrect, and fortunately that function is used infrequently 
enough that we can update its callers where appropriate, as here.


https://reviews.llvm.org/D91147

Files:
  clang/lib/Driver/ToolChain.cpp
  clang/test/Driver/frame-pointer-elim.c
  llvm/include/llvm/ADT/Triple.h
  llvm/lib/BinaryFormat/MachO.cpp


Index: llvm/lib/BinaryFormat/MachO.cpp
===
--- llvm/lib/BinaryFormat/MachO.cpp
+++ llvm/lib/BinaryFormat/MachO.cpp
@@ -55,7 +55,7 @@
 }
 
 static MachO::CPUSubTypeARM64 getARM64SubType(const Triple &T) {
-  assert(T.isAArch64() || T.getArch() == Triple::aarch64_32);
+  assert(T.isAArch64());
   if (T.isArch32Bit())
 return (MachO::CPUSubTypeARM64)MachO::CPU_SUBTYPE_ARM64_32_V8;
   if (T.getArchName() == "arm64e")
@@ -84,9 +84,7 @@
   if (T.isARM() || T.isThumb())
 return MachO::CPU_TYPE_ARM;
   if (T.isAArch64())
-return MachO::CPU_TYPE_ARM64;
-  if (T.getArch() == Triple::aarch64_32)
-return MachO::CPU_TYPE_ARM64_32;
+return T.isArch32Bit() ? MachO::CPU_TYPE_ARM64_32 : MachO::CPU_TYPE_ARM64;
   if (T.getArch() == Triple::ppc)
 return MachO::CPU_TYPE_POWERPC;
   if (T.getArch() == Triple::ppc64)
Index: llvm/include/llvm/ADT/Triple.h
===
--- llvm/include/llvm/ADT/Triple.h
+++ llvm/include/llvm/ADT/Triple.h
@@ -714,7 +714,8 @@
 
   /// Tests whether the target is AArch64 (little and big endian).
   bool isAArch64() const {
-return getArch() == Triple::aarch64 || getArch() == Triple::aarch64_be;
+return getArch() == Triple::aarch64 || getArch() == Triple::aarch64_be ||
+   getArch() == Triple::aarch64_32;
   }
 
   /// Tests whether the target is MIPS 32-bit (little and big endian).
Index: clang/test/Driver/frame-pointer-elim.c
===
--- clang/test/Driver/frame-pointer-elim.c
+++ clang/test/Driver/frame-pointer-elim.c
@@ -97,6 +97,8 @@
 // RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
 // RUN: %clang -### -target x86_64-scei-ps4 -S -O2 %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
+// RUN: %clang -### -target aarch64-apple-darwin -arch arm64_32 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
 
 // RUN: %clang -### -target powerpc64 -S %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=KEEP-ALL %s
Index: clang/lib/Driver/ToolChain.cpp
===
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -1063,9 +1063,10 @@
   getTriple().isAArch64())
 Res |= SanitizerKind::CFIICall;
   if (getTriple().getArch() == llvm::Triple::x86_64 ||
-  getTriple().isAArch64() || getTriple().isRISCV())
+  (getTriple().isAArch64() && getTriple().isArch64Bit()) ||
+  getTriple().isRISCV())
 Res |= SanitizerKind::ShadowCallStack;
-  if (getTriple().isAArch64())
+  if (getTriple().isAArch64() && getTriple().isArch64Bit())
 Res |= SanitizerKind::MemTag;
   return Res;
 }


Index: llvm/lib/BinaryFormat/MachO.cpp
===
--- llvm/lib/BinaryFormat/MachO.cpp
+++ llvm/lib/BinaryFormat/MachO.cpp
@@ -55,7 +55,7 @@
 }
 
 static MachO::CPUSubTypeARM64 getARM64SubType(const Triple &T) {
-  assert(T.isAArch64() || T.getArch() == Triple::aarch64_32);
+  assert(T.isAArch64());
   if (T.isArch32Bit())
 return (MachO::CPUSubTypeARM64)MachO::CPU_SUBTYPE_ARM64_32_V8;
   if (T.getArchName() == "arm64e")
@@ -84,9 +84,7 @@
   if (T.isARM() || T.isThumb())
 return MachO::CPU_TYPE_ARM;
   if (T.isAArch64())
-return MachO::CPU_TYPE_ARM64;
-  if (T.getArch() == Triple::aarch64_32)
-return MachO::CPU_TYPE_ARM64_32;
+return T.isArch32Bit() ? MachO::CPU_TYPE_ARM64_32 : MachO::CPU_TYPE_ARM64;
   if (T.getArch() == Triple::ppc)
 return MachO::CPU_TYPE_POWERPC;
   if (T.getArch() == Triple::ppc64)
Index: llvm/include/llvm/ADT/Triple.h
===
--- llvm/include/llvm/ADT/Triple.h
+++ llvm/include/llvm/ADT/Triple.h
@@ -714,7 +714,8 @@
 
   /// Tests whether the target is AArch64 (little and big endian).
   bool isAArch64() const {
-return getArch() == Triple::aarch64 || getArch() == Triple::aarch64_be;
+return getArch() == Triple::aarch64 || getArch() == Triple::aarch64_be ||
+   getArch() == Triple::aarch64_32;
   }
 
   /// Tests whether the target is MIPS 32-bit (little and big endian).
Index: clang/test/Driver/frame-pointer-elim.c
=

[PATCH] D91157: [AArch64] Out-of-line atomics (-moutline-atomics) implementation.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6366
+  // Option -moutline-atomics supported for AArch64 target only.
+  if (Triple.getArch() != llvm::Triple::aarch64) {
+D.Diag(diag::warn_drv_moutline_atomics_unsupported_opt)

This excludes `aarch64_be`, which is a bit weird. Might be best to check 
`Triple.isAArch64()`.



Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:1818
 
-  if (SrcSize > SlotSize) 
+  if (SrcSize > SlotSize)
 Store = DAG.getTruncStore(Chain, dl, SrcOp, FIPtr, PtrInfo,

Could you do the whitespace changes separately (if at all)?



Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:4064
+  // If outline atomic available
+  // prepare it's arguments and expand.
+  Ops.append(Node->op_begin() + 2, Node->op_end());

"its"



Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:2170
+  SmallVector Ops;
+  if (TLI.getLibcallName(LC)) {
+Ops.append(Node->op_begin() + 2, Node->op_end());

I think this is a bit of an abuse of the `LibcallName` mechanism. A separate 
function in `TargetLowering` would probably be better.



Comment at: llvm/lib/Target/AArch64/AArch64Subtarget.h:476
+  bool outlineAtomics() const {
+// Don't outline atomics if
+// subtarget has LSE

I think something is a bit weird with how your clang-format handles comments. 
Here and earlier line lengths are about half as long as I'd expect.



Comment at: llvm/test/CodeGen/AArch64/atomic-ops.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -mtriple=aarch64-none-linux-gnu -disable-post-ra 
-verify-machineinstrs < %s | FileCheck %s

I'd prefer not to overwrite existing CHECKS that have generic dataflow with 
ones produced by that tool hardcoding a particular register allocation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91157

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

[PATCH] D91157: [AArch64] Out-of-line atomics (-moutline-atomics) implementation.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added inline comments.



Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:2170
+  SmallVector Ops;
+  if (TLI.getLibcallName(LC)) {
+Ops.append(Node->op_begin() + 2, Node->op_end());

jyknight wrote:
> t.p.northover wrote:
> > I think this is a bit of an abuse of the `LibcallName` mechanism. A 
> > separate function in `TargetLowering` would probably be better.
> I don't think that's odd or unusual -- we often condition libcall 
> availability on getLibcallName != nullptr.
> 
> What does strike me here is the (pre-existing) code duplication between this 
> function (DAGTypeLegalizer::ExapndAtomic) and 
> SelectionDAGLegalize::ConvertNodeToLibcall. Not sure what's up with that...
Fair enough. Didn't realise it was that common.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91157

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

[PATCH] D91147: AArch64: classify Triple::aarch64_32 as AArch64

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added inline comments.



Comment at: clang/lib/Driver/ToolChain.cpp:1066
   if (getTriple().getArch() == llvm::Triple::x86_64 ||
-  getTriple().isAArch64() || getTriple().isRISCV())
+  (getTriple().isAArch64() && getTriple().isArch64Bit()) ||
+  getTriple().isRISCV())

dexonsmith wrote:
> Is there a short-form we'd want for this?
> 
> Here are two ideas:
> ```
> // getTriple().isAArch64_64() and getTriple().isAArch64_32()
> bool Triple::isAArch64_64() { return isAArch64() && isArch64Bit(); }
> bool Triple::isAArch64_32() { return isAArch64() && isArch32Bit(); }
> 
> // getTriple().isAArch64(64) and getTriple().isAArch64(32)
> bool Triple::isAArch64(int Bits) {
>   assert(Bits == 32 || Bits == 64);
>   if (!isAArch64())
> return false;
>   return isArch64Bit() ? Bits == 64 : Bits == 32;
> }
> ```
> Or do you think it's better as-is?
Tricky one. The bare "64" and "32" are a bit non-descript, but having both 
`isAArch64` and `isArch64` in the same line seems just slightly worse to me so 
I'll update the patch.


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

https://reviews.llvm.org/D91147

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

[PATCH] D91147: AArch64: classify Triple::aarch64_32 as AArch64

[Tim Northover via Phabricator via cfe-commits]

t.p.northover updated this revision to Diff 305064.

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

https://reviews.llvm.org/D91147

Files:
  clang/lib/Driver/ToolChain.cpp
  clang/test/Driver/frame-pointer-elim.c
  llvm/include/llvm/ADT/Triple.h
  llvm/lib/BinaryFormat/MachO.cpp


Index: llvm/lib/BinaryFormat/MachO.cpp
===
--- llvm/lib/BinaryFormat/MachO.cpp
+++ llvm/lib/BinaryFormat/MachO.cpp
@@ -55,7 +55,7 @@
 }
 
 static MachO::CPUSubTypeARM64 getARM64SubType(const Triple &T) {
-  assert(T.isAArch64() || T.getArch() == Triple::aarch64_32);
+  assert(T.isAArch64());
   if (T.isArch32Bit())
 return (MachO::CPUSubTypeARM64)MachO::CPU_SUBTYPE_ARM64_32_V8;
   if (T.getArchName() == "arm64e")
@@ -84,9 +84,7 @@
   if (T.isARM() || T.isThumb())
 return MachO::CPU_TYPE_ARM;
   if (T.isAArch64())
-return MachO::CPU_TYPE_ARM64;
-  if (T.getArch() == Triple::aarch64_32)
-return MachO::CPU_TYPE_ARM64_32;
+return T.isArch32Bit() ? MachO::CPU_TYPE_ARM64_32 : MachO::CPU_TYPE_ARM64;
   if (T.getArch() == Triple::ppc)
 return MachO::CPU_TYPE_POWERPC;
   if (T.getArch() == Triple::ppc64)
Index: llvm/include/llvm/ADT/Triple.h
===
--- llvm/include/llvm/ADT/Triple.h
+++ llvm/include/llvm/ADT/Triple.h
@@ -714,7 +714,17 @@
 
   /// Tests whether the target is AArch64 (little and big endian).
   bool isAArch64() const {
-return getArch() == Triple::aarch64 || getArch() == Triple::aarch64_be;
+return getArch() == Triple::aarch64 || getArch() == Triple::aarch64_be ||
+   getArch() == Triple::aarch64_32;
+  }
+
+  /// Tests whether the target is AArch64 and pointers are the size specified 
by
+  /// \p PointerWidth.
+  bool isAArch64(int PointerWidth) const {
+assert(PointerWidth == 64 || PointerWidth == 32);
+if (!isAArch64())
+  return false;
+return isArch64Bit() ? PointerWidth == 64 : PointerWidth == 32;
   }
 
   /// Tests whether the target is MIPS 32-bit (little and big endian).
Index: clang/test/Driver/frame-pointer-elim.c
===
--- clang/test/Driver/frame-pointer-elim.c
+++ clang/test/Driver/frame-pointer-elim.c
@@ -97,6 +97,8 @@
 // RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
 // RUN: %clang -### -target x86_64-scei-ps4 -S -O2 %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
+// RUN: %clang -### -target aarch64-apple-darwin -arch arm64_32 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NON-LEAF %s
 
 // RUN: %clang -### -target powerpc64 -S %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=KEEP-ALL %s
Index: clang/lib/Driver/ToolChain.cpp
===
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -1063,9 +1063,9 @@
   getTriple().isAArch64())
 Res |= SanitizerKind::CFIICall;
   if (getTriple().getArch() == llvm::Triple::x86_64 ||
-  getTriple().isAArch64() || getTriple().isRISCV())
+  getTriple().isAArch64(64) || getTriple().isRISCV())
 Res |= SanitizerKind::ShadowCallStack;
-  if (getTriple().isAArch64())
+  if (getTriple().isAArch64(64))
 Res |= SanitizerKind::MemTag;
   return Res;
 }


Index: llvm/lib/BinaryFormat/MachO.cpp
===
--- llvm/lib/BinaryFormat/MachO.cpp
+++ llvm/lib/BinaryFormat/MachO.cpp
@@ -55,7 +55,7 @@
 }
 
 static MachO::CPUSubTypeARM64 getARM64SubType(const Triple &T) {
-  assert(T.isAArch64() || T.getArch() == Triple::aarch64_32);
+  assert(T.isAArch64());
   if (T.isArch32Bit())
 return (MachO::CPUSubTypeARM64)MachO::CPU_SUBTYPE_ARM64_32_V8;
   if (T.getArchName() == "arm64e")
@@ -84,9 +84,7 @@
   if (T.isARM() || T.isThumb())
 return MachO::CPU_TYPE_ARM;
   if (T.isAArch64())
-return MachO::CPU_TYPE_ARM64;
-  if (T.getArch() == Triple::aarch64_32)
-return MachO::CPU_TYPE_ARM64_32;
+return T.isArch32Bit() ? MachO::CPU_TYPE_ARM64_32 : MachO::CPU_TYPE_ARM64;
   if (T.getArch() == Triple::ppc)
 return MachO::CPU_TYPE_POWERPC;
   if (T.getArch() == Triple::ppc64)
Index: llvm/include/llvm/ADT/Triple.h
===
--- llvm/include/llvm/ADT/Triple.h
+++ llvm/include/llvm/ADT/Triple.h
@@ -714,7 +714,17 @@
 
   /// Tests whether the target is AArch64 (little and big endian).
   bool isAArch64() const {
-return getArch() == Triple::aarch64 || getArch() == Triple::aarch64_be;
+return getArch() == Triple::aarch64 || getArch() == Triple::aarch64_be ||
+   getArch() == Triple::aarch64_32;
+  }
+
+  /// Tests whether the target is AArch64 and pointers are the size specified by
+  /// \p PointerWidth.
+  bool isAArch64(int PointerWidth) const {
+assert(PointerWidth == 64 || PointerWidth == 32);
+if (!isAArch64())
+  return false;
+return isArch64Bit()

[PATCH] D91147: AArch64: classify Triple::aarch64_32 as AArch64

[Tim Northover via Phabricator via cfe-commits]

t.p.northover closed this revision.
t.p.northover added a comment.

Thanks Gerolf. Committed:

To github.com:llvm/llvm-project.git

  ae9d96a656a1..152df3add156  master -> master


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

https://reviews.llvm.org/D91147

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

[PATCH] D95228: Add swift_async_context parameter attribute mapping to IR equivalent

[Tim Northover via Phabricator via cfe-commits]

t.p.northover closed this revision.
t.p.northover marked an inline comment as done.
t.p.northover added a comment.

Thanks, committed as e94fada045fe 
 with the 
extra tests.




Comment at: clang/include/clang/Basic/AttrDocs.td:4404
+
+A context parameter must have pointer or reference type.
+  }];

aaron.ballman wrote:
> Should we specify whether pointer to members or ObjC object pointers are fine 
> (given that those are both somewhat "odd" pointer types)? Function pointers?
> 
> (We may want to clarify this in the other docs in a follow-up if we think 
> this is unclear.)
I don't think either of those are banned, but it's kind of a "play silly games 
win silly prizes" situation.


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

https://reviews.llvm.org/D95228

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

[PATCH] D103165: Threading: use independent llvm::thread implementation on Apple platforms to increase stack size

[Tim Northover via Phabricator via cfe-commits]

t.p.northover updated this revision to Diff 348958.
t.p.northover added a comment.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, hiraditya.
Herald added a project: clang-tools-extra.

- Add `llvm::thread` for all potential platforms, allowing stack size to be 
specified.
- `llvm::thread` based on the `Threading.inc` where possible, else 
`std::thread`, else in-thread synchronous execution (with progressively more 
failures).
- Remove `llvm_execute_on_thread*` in favour of this new `llvm::thread`.

I've managed to test it on all 3 major platforms, and the fallbacks by fiddling 
`#if`s. So it should at least compile everywhere.

> Also this way llvm::thread users that don't need full-sized stacks can 
> continue getting the Darwin default of smaller pages.

I'm not sure I've implemented this, since I've just made 8MB the Darwin default 
here. The case I care about is LTOBackend via `ThreadPool`. Would you prefer me 
to push an optional `StackSize` argument into `ThreadPool` too and make 
LTOBackend specify it?

I kind of think this has come up often enough (it's at least the second time 
I've had to fix it somewhere), and 8MB is small enough for anything running 
Clang that it's better to make sure it doesn't happen again.


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

https://reviews.llvm.org/D103165

Files:
  clang-tools-extra/clangd/support/Threading.cpp
  clang/tools/libclang/CIndex.cpp
  llvm/include/llvm/Support/CrashRecoveryContext.h
  llvm/include/llvm/Support/Threading.h
  llvm/include/llvm/Support/thread.h
  llvm/lib/Support/CrashRecoveryContext.cpp
  llvm/lib/Support/Threading.cpp
  llvm/lib/Support/Unix/Threading.inc
  llvm/lib/Support/Windows/Threading.inc
  llvm/unittests/Support/Threading.cpp

Index: llvm/unittests/Support/Threading.cpp
===
--- llvm/unittests/Support/Threading.cpp
+++ llvm/unittests/Support/Threading.cpp
@@ -63,7 +63,8 @@
 ThreadFinished.notify();
   };
 
-  llvm::llvm_execute_on_thread_async(ThreadFunc);
+  llvm::thread Thread(ThreadFunc);
+  Thread.detach();
   ASSERT_TRUE(ThreadStarted.wait());
   ThreadAdvanced.notify();
   ASSERT_TRUE(ThreadFinished.wait());
@@ -71,11 +72,23 @@
 
 TEST(Threading, RunOnThreadSync) {
   std::atomic_bool Executed(false);
-  llvm::llvm_execute_on_thread(
+  llvm::thread Thread(
   [](void *Arg) { *static_cast(Arg) = true; },
   &Executed);
+  Thread.join();
   ASSERT_EQ(Executed, true);
 }
+
+#if defined(__APPLE__)
+TEST(Threading, AppleStackSize) {
+  llvm::thread Thread([] {
+volatile unsigned char Var[8 * 1024 * 1024 - 1024];
+Var[0] = 0xff;
+ASSERT_EQ(Var[0], 0xff);
+  });
+  Thread.join();
+}
+#endif
 #endif
 
 } // end anon namespace
Index: llvm/lib/Support/Windows/Threading.inc
===
--- llvm/lib/Support/Windows/Threading.inc
+++ llvm/lib/Support/Windows/Threading.inc
@@ -23,22 +23,10 @@
 #undef MemoryFence
 #endif
 
-static unsigned __stdcall threadFuncSync(void *Arg) {
-  SyncThreadInfo *TI = static_cast(Arg);
-  TI->UserFn(TI->UserData);
-  return 0;
-}
-
-static unsigned __stdcall threadFuncAsync(void *Arg) {
-  std::unique_ptr Info(static_cast(Arg));
-  (*Info)();
-  return 0;
-}
-
-static void
-llvm_execute_on_thread_impl(unsigned (__stdcall *ThreadFunc)(void *), void *Arg,
-llvm::Optional StackSizeInBytes,
-JoiningPolicy JP) {
+HANDLE
+llvm::llvm_execute_on_thread_impl(unsigned(__stdcall *ThreadFunc)(void *),
+  void *Arg,
+  llvm::Optional StackSizeInBytes) {
   HANDLE hThread = (HANDLE)::_beginthreadex(
   NULL, StackSizeInBytes.getValueOr(0), ThreadFunc, Arg, 0, NULL);
 
@@ -46,11 +34,16 @@
 ReportLastErrorFatal("_beginthreadex failed");
   }
 
-  if (JP == JoiningPolicy::Join) {
-if (::WaitForSingleObject(hThread, INFINITE) == WAIT_FAILED) {
-  ReportLastErrorFatal("WaitForSingleObject failed");
-}
+  return hThread;
+}
+
+void llvm::llvm_thread_join_impl(HANDLE hThread) {
+  if (::WaitForSingleObject(hThread, INFINITE) == WAIT_FAILED) {
+ReportLastErrorFatal("WaitForSingleObject failed");
   }
+}
+
+void llvm::llvm_thread_detach_impl(HANDLE hThread) {
   if (::CloseHandle(hThread) == FALSE) {
 ReportLastErrorFatal("CloseHandle failed");
   }
Index: llvm/lib/Support/Unix/Threading.inc
===
--- llvm/lib/Support/Unix/Threading.inc
+++ llvm/lib/Support/Unix/Threading.inc
@@ -48,22 +48,9 @@
 #include   // For syscall()
 #endif
 
-static void *threadFuncSync(void *Arg) {
-  SyncThreadInfo *TI = static_cast(Arg);
-  TI->UserFn(TI->UserData);
-  return nullptr;
-}
-
-static void *threadFuncAsync(void *Arg) {
-  std::unique_ptr Info(static_cast(Arg));
-  (*Info)();
-  return nullptr;
-}
-
-static void
-llvm_execute_on_thread_impl

[PATCH] D103165: Threading: use independent llvm::thread implementation on Apple platforms to increase stack size

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added inline comments.



Comment at: llvm/include/llvm/Support/thread.h:168
+/// stack size request.
+class thread {
+public:

An alternative here would have been to inherit from `std::thread` but that 
seemed a bit icky to me. Happy to switch if the consensus is otherwise.


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

https://reviews.llvm.org/D103165

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

[PATCH] D95228: Add swift_async_context parameter attribute mapping to IR equivalent

[Tim Northover via Phabricator via cfe-commits]

t.p.northover created this revision.
Herald added subscribers: dexonsmith, jdoerfert, mcrosier.
Herald added a reviewer: aaron.ballman.
t.p.northover requested review of this revision.

This adds `__attribute__((swift_async_context))` very much following the 
existing `swift_context` so that Swift's runtime implementation in C++ can 
generate functions that interact with Swift code properly. Hopefully nothing 
too controversial.

It depends on D95044  for the IR support.


https://reviews.llvm.org/D95228

Files:
  clang/include/clang/AST/Attr.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/Specifiers.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/arm-swiftcall.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/attr-swiftcall.c

Index: clang/test/Sema/attr-swiftcall.c
===
--- clang/test/Sema/attr-swiftcall.c
+++ clang/test/Sema/attr-swiftcall.c
@@ -5,6 +5,7 @@
 #define INDIRECT_RESULT __attribute__((swift_indirect_result))
 #define ERROR_RESULT __attribute__((swift_error_result))
 #define CONTEXT __attribute__((swift_context))
+#define ASYNC_CONTEXT __attribute__((swift_async_context))
 
 int notAFunction SWIFTCALL; // expected-warning {{'swiftcall' only applies to function types; type here is 'int'}}
 void variadic(int x, ...) SWIFTCALL; // expected-error {{variadic function cannot use swiftcall calling convention}}
@@ -29,3 +30,8 @@
 void context_bad_type(CONTEXT int context) SWIFTCALL; // expected-error {{'swift_context' parameter must have pointer type; type here is 'int'}}
 void context_okay(CONTEXT void *context) SWIFTCALL;
 void context_okay2(CONTEXT void *context, void *selfType, char **selfWitnessTable) SWIFTCALL;
+
+void async_context_okay_for_now(ASYNC_CONTEXT void *context);
+void async_context_bad_type(ASYNC_CONTEXT int context) SWIFTCALL; // expected-error {{'swift_async_context' parameter must have pointer type; type here is 'int'}}
+void async_context_okay(ASYNC_CONTEXT void *context) SWIFTCALL;
+void async_context_okay2(ASYNC_CONTEXT void *context, void *selfType, char **selfWitnessTable) SWIFTCALL;
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -154,6 +154,7 @@
 // CHECK-NEXT: SetTypestate (SubjectMatchRule_function_is_member)
 // CHECK-NEXT: SpeculativeLoadHardening (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: SwiftAsync (SubjectMatchRule_function, SubjectMatchRule_objc_method)
+// CHECK-NEXT: SwiftAsyncContext (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: SwiftAsyncName (SubjectMatchRule_objc_method, SubjectMatchRule_function)
 // CHECK-NEXT: SwiftBridgedTypedef (SubjectMatchRule_type_alias)
 // CHECK-NEXT: SwiftContext (SubjectMatchRule_variable_is_parameter)
Index: clang/test/CodeGen/arm-swiftcall.c
===
--- clang/test/CodeGen/arm-swiftcall.c
+++ clang/test/CodeGen/arm-swiftcall.c
@@ -6,6 +6,7 @@
 #define OUT __attribute__((swift_indirect_result))
 #define ERROR __attribute__((swift_error_result))
 #define CONTEXT __attribute__((swift_context))
+#define ASYNC_CONTEXT __attribute__((swift_async_context))
 
 /*/
 /** PARAMETER ABIS ***/
@@ -53,6 +54,9 @@
 SWIFTCALL void context_error_2(short s, CONTEXT int *self, ERROR float **error) {}
 // CHECK-LABEL: define{{.*}} void @context_error_2(i16{{.*}}, i32* swiftself{{.*}}, float** swifterror %0)
 
+SWIFTCALL void async_context_1(ASYNC_CONTEXT void *self) {}
+// CHECK-LABEL: define {{.*}} void @async_context_1(i8* swiftasync
+
 /*/
 /** LOWERING */
 /*/
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2791,6 +2791,10 @@
   checkForSwiftCC(paramIndex);
   continue;
 
+case ParameterABI::SwiftAsyncContext:
+  // FIXME: might want to require swiftasynccc when it exists
+  continue;
+
 // swift_error parameters must be preceded by a swift_context parameter.
 case ParameterABI::SwiftErrorResult:
   checkForSwiftCC(paramIndex);
Index: clang/lib/Sema/SemaDeclAttr.cpp
=