subject:"\[PATCH\] D79118\: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs"

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

2020-05-30 Thread Erich Keane via Phabricator via cfe-commits

erichkeane closed this revision.
erichkeane added a comment.
Herald added a subscriber: sstefan1.

Yikes, I apparently misspelled "Revision" as "revisions"!.
https://github.com/llvm/llvm-project/commit/8a1c999c9b0817d4de778a62965b4af86416e4b7


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

https://reviews.llvm.org/D79118



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

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

2020-05-06 Thread Erich Keane via Phabricator via cfe-commits

erichkeane updated this revision to Diff 262274.
erichkeane marked 5 inline comments as done.

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

https://reviews.llvm.org/D79118

Files:
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Basic/Targets/ARC.h
  clang/lib/Basic/Targets/ARM.h
  clang/lib/Basic/Targets/Hexagon.h
  clang/lib/Basic/Targets/Lanai.h
  clang/lib/Basic/Targets/Mips.h
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/Basic/Targets/PNaCl.h
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/Basic/Targets/SPIR.h
  clang/lib/Basic/Targets/Sparc.h
  clang/lib/Basic/Targets/SystemZ.h
  clang/lib/Basic/Targets/WebAssembly.h
  clang/lib/Basic/Targets/XCore.h
  clang/lib/CodeGen/ABIInfo.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/ext-int-cc.c
  clang/test/CodeGen/ext-int-sanitizer.cpp
  clang/test/CodeGenCXX/ext-int.cpp
  clang/test/Sema/ext-int-not-supported.c

Index: clang/test/Sema/ext-int-not-supported.c
===
--- clang/test/Sema/ext-int-not-supported.c
+++ /dev/null
@@ -1,5 +0,0 @@
-// RUN: %clang_cc1 -triple armv7 -fsyntax-only -verify %s
-
-void foo() {
-  _ExtInt(33) a; // expected-error{{_ExtInt is not supported on this target}}
-}
Index: clang/test/CodeGenCXX/ext-int.cpp
===
--- clang/test/CodeGenCXX/ext-int.cpp
+++ clang/test/CodeGenCXX/ext-int.cpp
@@ -98,7 +98,7 @@
 };
 
 void UnderlyingTypeUsage(AsEnumUnderlyingType Param) {
-  // LIN: define void @_Z19UnderlyingTypeUsage20AsEnumUnderlyingType(i16 %
+  // LIN: define void @_Z19UnderlyingTypeUsage20AsEnumUnderlyingType(i9 signext %
   // WIN: define dso_local void @"?UnderlyingTypeUsage@@YAXW4AsEnumUnderlyingType@@@Z"(i9 %
   AsEnumUnderlyingType Var;
   // CHECK: alloca i9, align 2
Index: clang/test/CodeGen/ext-int-sanitizer.cpp
===
--- clang/test/CodeGen/ext-int-sanitizer.cpp
+++ clang/test/CodeGen/ext-int-sanitizer.cpp
@@ -145,8 +145,7 @@
 
   // Also triggers signed integer overflow.
   E / E;
-  // CHECK: %[[E1LOAD:.+]] = load i11
-  // CHECK: store i11 %[[E1LOAD]], i11* %[[EADDR:.+]]
+  // CHECK: %[[EADDR:.+]] = alloca i11
   // CHECK: %[[E:.+]] = load i11, i11* %[[EADDR]]
   // CHECK: %[[E2:.+]] = load i11, i11* %[[EADDR]]
   // CHECK: %[[NEZERO:.+]] = icmp ne i11 %[[E2]], 0
@@ -163,8 +162,7 @@
 // CHECK: define void @_Z6ShiftsU7_ExtIntILi9EEi
 void Shifts(_ExtInt(9) E) {
   E >> E;
-  // CHECK: %[[E1LOAD:.+]] = load i9, i9*
-  // CHECK: store i9 %[[E1LOAD]], i9* %[[EADDR:.+]]
+  // CHECK: %[[EADDR:.+]] = alloca i9
   // CHECK: %[[LHSE:.+]] = load i9, i9* %[[EADDR]]
   // CHECK: %[[RHSE:.+]] = load i9, i9* %[[EADDR]]
   // CHECK: %[[CMP:.+]] = icmp ule i9 %[[RHSE]], 8
@@ -227,8 +225,7 @@
  unsigned _ExtInt(23) SmallE,
  unsigned _ExtInt(35) BigE) {
   u = SmallE + SmallE;
-  // CHECK: %[[LOADBIGGESTE2:.+]] = load i23
-  // CHECK: store i23 %[[LOADBIGGESTE2]], i23* %[[BIGGESTEADDR:.+]]
+  // CHECK: %[[BIGGESTEADDR:.+]] = alloca i23
   // CHECK: %[[LOADE1:.+]] = load i23, i23* %[[BIGGESTEADDR]]
   // CHECK: %[[LOADE2:.+]] = load i23, i23* %[[BIGGESTEADDR]]
   // CHECK: %[[OFCALL:.+]] = call { i23, i1 } @llvm.uadd.with.overflow.i23(i23 %[[LOADE1]], i23 %[[LOADE2]])
Index: clang/test/CodeGen/ext-int-cc.c
===
--- clang/test/CodeGen/ext-int-cc.c
+++ clang/test/CodeGen/ext-int-cc.c
@@ -2,6 +2,32 @@
 // RUN: %clang_cc1 -triple x86_64-windows-pc -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=WIN64
 // RUN: %clang_cc1 -triple i386-gnu-linux -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=LIN32
 // RUN: %clang_cc1 -triple i386-windows-pc -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=WIN32
+// RUN: %clang_cc1 -triple le32-nacl -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=NACL
+// RUN: %clang_cc1 -triple nvptx64 -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=NVPTX64
+// RUN: %clang_cc1 -triple nvptx -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=NVPTX
+// RUN: %clang_cc1 -triple sparcv9 -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=SPARCV9
+// RUN: %clang_cc1 -triple sparc -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=SPARC
+// RUN: %clang_cc1 -triple mips64 -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=MIPS64
+// RUN: %clang_cc1 -triple mips -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=MIPS
+// RUN: %clang_cc1 -triple spir64 -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=SPIR64
+// RUN: %clang_cc1 -triple spir -O3

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

2020-05-06 Thread John McCall via Phabricator via cfe-commits

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

Okay, LGTM.


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

https://reviews.llvm.org/D79118



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

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

2020-05-05 Thread John McCall via Phabricator via cfe-commits

rjmccall added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:719
+ !getContext().getTargetInfo().hasInt128Type()))
+  return getNaturalAlignIndirect(Ty);
+

erichkeane wrote:
> rjmccall wrote:
> > It's very weird for 64 and 128 to be showing up as constants in the default 
> > ABI rule.
> Good point.  I rewrote this in terms of the Context.LongLongTy and 
> Context.Int128Ty.
Maybe:

```
if (EIT->getNumBits() > 
Context.getTypeSize(Context.getTargetInfo().hasInt128Type() ? Context.Int128Ty 
: Context.LongLongTy)))
```



Comment at: clang/lib/CodeGen/TargetInfo.cpp:741
+ !getContext().getTargetInfo().hasInt128Type()))
+  return getNaturalAlignIndirect(RetTy);
+

Same rule needed here.


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

https://reviews.llvm.org/D79118



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

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

2020-05-05 Thread Erich Keane via Phabricator via cfe-commits

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



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4938
+if (EIT->getNumBits() > 128)
+  return getNaturalAlignIndirect(Ty, /*ByVal=*/true);
+

rjmccall wrote:
> erichkeane wrote:
> > rjmccall wrote:
> > > rjmccall wrote:
> > > > erichkeane wrote:
> > > > > rjmccall wrote:
> > > > > > Does this need to consider the aggregate-as-array logic below?
> > > > > I'm not sure what you mean by this?  Are you suggesting I 
> > > > > could/should pass this as an array instead of indirectly?
> > > > My interpretation is that in general you're lowering large a `_ExtInt` 
> > > > like a struct with a bunch of integer members in it.  My understanding 
> > > > is that, for this target, that would make it a homogeneous aggregate 
> > > > eligible for the special treatment given to certain aggregate types 
> > > > below.  I don't know what that corresponds to at the code-emission 
> > > > level.
> > > This seems to work, although it's unfortunate that it duplicates some of 
> > > the existing behavior.  Do we also need to modify 
> > > `isHomogeneousAggregate` to consider `ExtInts`?  And if we do, does that 
> > > avoid any of the duplicate logic here?
> > I think doing that would result in altering a number of other targets as 
> > well (thats not a PPC specific function I think?). 
> > 
> > While there IS a little duplication, it is just the 5 lines that calculates 
> > the array and coerces it. Otherwise the logic is just slightly different 
> > enough I don't think it would save much.
> > 
> > That said, perhaps there is value in extracting those 5 lines or so into a 
> > function.  I'll give that a shot.
> Yeah, the function isn't PPC-specific.  The question is what the right rules 
> are for homogeneous aggregates when aggregates contain ExtInt types.  For 
> PPC64 ELFv2, that... oh right, it only applies to floating-point and vector 
> types.  So I'm sorry, I led you on a false path here; we should not be trying 
> to apply this logic to ExtInt types at all.
So I should switch it back to direct/indirect like earlier?  That I can do.


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

https://reviews.llvm.org/D79118



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

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

2020-05-05 Thread Erich Keane via Phabricator via cfe-commits

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

Revert PPC64 changes back to direct/indirect.


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

https://reviews.llvm.org/D79118

Files:
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Basic/Targets/ARC.h
  clang/lib/Basic/Targets/ARM.h
  clang/lib/Basic/Targets/Hexagon.h
  clang/lib/Basic/Targets/Lanai.h
  clang/lib/Basic/Targets/Mips.h
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/Basic/Targets/PNaCl.h
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/Basic/Targets/SPIR.h
  clang/lib/Basic/Targets/Sparc.h
  clang/lib/Basic/Targets/SystemZ.h
  clang/lib/Basic/Targets/WebAssembly.h
  clang/lib/Basic/Targets/XCore.h
  clang/lib/CodeGen/ABIInfo.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/ext-int-cc.c
  clang/test/CodeGen/ext-int-sanitizer.cpp
  clang/test/CodeGenCXX/ext-int.cpp
  clang/test/Sema/ext-int-not-supported.c

Index: clang/test/Sema/ext-int-not-supported.c
===
--- clang/test/Sema/ext-int-not-supported.c
+++ /dev/null
@@ -1,5 +0,0 @@
-// RUN: %clang_cc1 -triple armv7 -fsyntax-only -verify %s
-
-void foo() {
-  _ExtInt(33) a; // expected-error{{_ExtInt is not supported on this target}}
-}
Index: clang/test/CodeGenCXX/ext-int.cpp
===
--- clang/test/CodeGenCXX/ext-int.cpp
+++ clang/test/CodeGenCXX/ext-int.cpp
@@ -98,7 +98,7 @@
 };
 
 void UnderlyingTypeUsage(AsEnumUnderlyingType Param) {
-  // LIN: define void @_Z19UnderlyingTypeUsage20AsEnumUnderlyingType(i16 %
+  // LIN: define void @_Z19UnderlyingTypeUsage20AsEnumUnderlyingType(i9 signext %
   // WIN: define dso_local void @"?UnderlyingTypeUsage@@YAXW4AsEnumUnderlyingType@@@Z"(i9 %
   AsEnumUnderlyingType Var;
   // CHECK: alloca i9, align 2
Index: clang/test/CodeGen/ext-int-sanitizer.cpp
===
--- clang/test/CodeGen/ext-int-sanitizer.cpp
+++ clang/test/CodeGen/ext-int-sanitizer.cpp
@@ -145,8 +145,7 @@
 
   // Also triggers signed integer overflow.
   E / E;
-  // CHECK: %[[E1LOAD:.+]] = load i11
-  // CHECK: store i11 %[[E1LOAD]], i11* %[[EADDR:.+]]
+  // CHECK: %[[EADDR:.+]] = alloca i11
   // CHECK: %[[E:.+]] = load i11, i11* %[[EADDR]]
   // CHECK: %[[E2:.+]] = load i11, i11* %[[EADDR]]
   // CHECK: %[[NEZERO:.+]] = icmp ne i11 %[[E2]], 0
@@ -163,8 +162,7 @@
 // CHECK: define void @_Z6ShiftsU7_ExtIntILi9EEi
 void Shifts(_ExtInt(9) E) {
   E >> E;
-  // CHECK: %[[E1LOAD:.+]] = load i9, i9*
-  // CHECK: store i9 %[[E1LOAD]], i9* %[[EADDR:.+]]
+  // CHECK: %[[EADDR:.+]] = alloca i9
   // CHECK: %[[LHSE:.+]] = load i9, i9* %[[EADDR]]
   // CHECK: %[[RHSE:.+]] = load i9, i9* %[[EADDR]]
   // CHECK: %[[CMP:.+]] = icmp ule i9 %[[RHSE]], 8
@@ -227,8 +225,7 @@
  unsigned _ExtInt(23) SmallE,
  unsigned _ExtInt(35) BigE) {
   u = SmallE + SmallE;
-  // CHECK: %[[LOADBIGGESTE2:.+]] = load i23
-  // CHECK: store i23 %[[LOADBIGGESTE2]], i23* %[[BIGGESTEADDR:.+]]
+  // CHECK: %[[BIGGESTEADDR:.+]] = alloca i23
   // CHECK: %[[LOADE1:.+]] = load i23, i23* %[[BIGGESTEADDR]]
   // CHECK: %[[LOADE2:.+]] = load i23, i23* %[[BIGGESTEADDR]]
   // CHECK: %[[OFCALL:.+]] = call { i23, i1 } @llvm.uadd.with.overflow.i23(i23 %[[LOADE1]], i23 %[[LOADE2]])
Index: clang/test/CodeGen/ext-int-cc.c
===
--- clang/test/CodeGen/ext-int-cc.c
+++ clang/test/CodeGen/ext-int-cc.c
@@ -2,6 +2,32 @@
 // RUN: %clang_cc1 -triple x86_64-windows-pc -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=WIN64
 // RUN: %clang_cc1 -triple i386-gnu-linux -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=LIN32
 // RUN: %clang_cc1 -triple i386-windows-pc -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=WIN32
+// RUN: %clang_cc1 -triple le32-nacl -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=NACL
+// RUN: %clang_cc1 -triple nvptx64 -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=NVPTX64
+// RUN: %clang_cc1 -triple nvptx -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=NVPTX
+// RUN: %clang_cc1 -triple sparcv9 -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=SPARCV9
+// RUN: %clang_cc1 -triple sparc -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=SPARC
+// RUN: %clang_cc1 -triple mips64 -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=MIPS64
+// RUN: %clang_cc1 -triple mips -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=MIPS
+// RUN: %clang_cc1 -triple spir64 -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=SPIR64
+// RUN: %clang_cc1

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

2020-05-05 Thread John McCall via Phabricator via cfe-commits

rjmccall added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4938
+if (EIT->getNumBits() > 128)
+  return getNaturalAlignIndirect(Ty, /*ByVal=*/true);
+

erichkeane wrote:
> rjmccall wrote:
> > rjmccall wrote:
> > > erichkeane wrote:
> > > > rjmccall wrote:
> > > > > Does this need to consider the aggregate-as-array logic below?
> > > > I'm not sure what you mean by this?  Are you suggesting I could/should 
> > > > pass this as an array instead of indirectly?
> > > My interpretation is that in general you're lowering large a `_ExtInt` 
> > > like a struct with a bunch of integer members in it.  My understanding is 
> > > that, for this target, that would make it a homogeneous aggregate 
> > > eligible for the special treatment given to certain aggregate types 
> > > below.  I don't know what that corresponds to at the code-emission level.
> > This seems to work, although it's unfortunate that it duplicates some of 
> > the existing behavior.  Do we also need to modify `isHomogeneousAggregate` 
> > to consider `ExtInts`?  And if we do, does that avoid any of the duplicate 
> > logic here?
> I think doing that would result in altering a number of other targets as well 
> (thats not a PPC specific function I think?). 
> 
> While there IS a little duplication, it is just the 5 lines that calculates 
> the array and coerces it. Otherwise the logic is just slightly different 
> enough I don't think it would save much.
> 
> That said, perhaps there is value in extracting those 5 lines or so into a 
> function.  I'll give that a shot.
Yeah, the function isn't PPC-specific.  The question is what the right rules 
are for homogeneous aggregates when aggregates contain ExtInt types.  For PPC64 
ELFv2, that... oh right, it only applies to floating-point and vector types.  
So I'm sorry, I led you on a false path here; we should not be trying to apply 
this logic to ExtInt types at all.


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

https://reviews.llvm.org/D79118



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

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

2020-05-05 Thread Erich Keane via Phabricator via cfe-commits

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

Extracted out a function for PPC64's coerce to array.


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

https://reviews.llvm.org/D79118

Files:
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Basic/Targets/ARC.h
  clang/lib/Basic/Targets/ARM.h
  clang/lib/Basic/Targets/Hexagon.h
  clang/lib/Basic/Targets/Lanai.h
  clang/lib/Basic/Targets/Mips.h
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/Basic/Targets/PNaCl.h
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/Basic/Targets/SPIR.h
  clang/lib/Basic/Targets/Sparc.h
  clang/lib/Basic/Targets/SystemZ.h
  clang/lib/Basic/Targets/WebAssembly.h
  clang/lib/Basic/Targets/XCore.h
  clang/lib/CodeGen/ABIInfo.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/ext-int-cc.c
  clang/test/CodeGen/ext-int-sanitizer.cpp
  clang/test/CodeGenCXX/ext-int.cpp
  clang/test/Sema/ext-int-not-supported.c

Index: clang/test/Sema/ext-int-not-supported.c
===
--- clang/test/Sema/ext-int-not-supported.c
+++ /dev/null
@@ -1,5 +0,0 @@
-// RUN: %clang_cc1 -triple armv7 -fsyntax-only -verify %s
-
-void foo() {
-  _ExtInt(33) a; // expected-error{{_ExtInt is not supported on this target}}
-}
Index: clang/test/CodeGenCXX/ext-int.cpp
===
--- clang/test/CodeGenCXX/ext-int.cpp
+++ clang/test/CodeGenCXX/ext-int.cpp
@@ -98,7 +98,7 @@
 };
 
 void UnderlyingTypeUsage(AsEnumUnderlyingType Param) {
-  // LIN: define void @_Z19UnderlyingTypeUsage20AsEnumUnderlyingType(i16 %
+  // LIN: define void @_Z19UnderlyingTypeUsage20AsEnumUnderlyingType(i9 signext %
   // WIN: define dso_local void @"?UnderlyingTypeUsage@@YAXW4AsEnumUnderlyingType@@@Z"(i9 %
   AsEnumUnderlyingType Var;
   // CHECK: alloca i9, align 2
Index: clang/test/CodeGen/ext-int-sanitizer.cpp
===
--- clang/test/CodeGen/ext-int-sanitizer.cpp
+++ clang/test/CodeGen/ext-int-sanitizer.cpp
@@ -145,8 +145,7 @@
 
   // Also triggers signed integer overflow.
   E / E;
-  // CHECK: %[[E1LOAD:.+]] = load i11
-  // CHECK: store i11 %[[E1LOAD]], i11* %[[EADDR:.+]]
+  // CHECK: %[[EADDR:.+]] = alloca i11
   // CHECK: %[[E:.+]] = load i11, i11* %[[EADDR]]
   // CHECK: %[[E2:.+]] = load i11, i11* %[[EADDR]]
   // CHECK: %[[NEZERO:.+]] = icmp ne i11 %[[E2]], 0
@@ -163,8 +162,7 @@
 // CHECK: define void @_Z6ShiftsU7_ExtIntILi9EEi
 void Shifts(_ExtInt(9) E) {
   E >> E;
-  // CHECK: %[[E1LOAD:.+]] = load i9, i9*
-  // CHECK: store i9 %[[E1LOAD]], i9* %[[EADDR:.+]]
+  // CHECK: %[[EADDR:.+]] = alloca i9
   // CHECK: %[[LHSE:.+]] = load i9, i9* %[[EADDR]]
   // CHECK: %[[RHSE:.+]] = load i9, i9* %[[EADDR]]
   // CHECK: %[[CMP:.+]] = icmp ule i9 %[[RHSE]], 8
@@ -227,8 +225,7 @@
  unsigned _ExtInt(23) SmallE,
  unsigned _ExtInt(35) BigE) {
   u = SmallE + SmallE;
-  // CHECK: %[[LOADBIGGESTE2:.+]] = load i23
-  // CHECK: store i23 %[[LOADBIGGESTE2]], i23* %[[BIGGESTEADDR:.+]]
+  // CHECK: %[[BIGGESTEADDR:.+]] = alloca i23
   // CHECK: %[[LOADE1:.+]] = load i23, i23* %[[BIGGESTEADDR]]
   // CHECK: %[[LOADE2:.+]] = load i23, i23* %[[BIGGESTEADDR]]
   // CHECK: %[[OFCALL:.+]] = call { i23, i1 } @llvm.uadd.with.overflow.i23(i23 %[[LOADE1]], i23 %[[LOADE2]])
Index: clang/test/CodeGen/ext-int-cc.c
===
--- clang/test/CodeGen/ext-int-cc.c
+++ clang/test/CodeGen/ext-int-cc.c
@@ -2,6 +2,32 @@
 // RUN: %clang_cc1 -triple x86_64-windows-pc -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=WIN64
 // RUN: %clang_cc1 -triple i386-gnu-linux -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=LIN32
 // RUN: %clang_cc1 -triple i386-windows-pc -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=WIN32
+// RUN: %clang_cc1 -triple le32-nacl -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=NACL
+// RUN: %clang_cc1 -triple nvptx64 -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=NVPTX64
+// RUN: %clang_cc1 -triple nvptx -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=NVPTX
+// RUN: %clang_cc1 -triple sparcv9 -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=SPARCV9
+// RUN: %clang_cc1 -triple sparc -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=SPARC
+// RUN: %clang_cc1 -triple mips64 -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=MIPS64
+// RUN: %clang_cc1 -triple mips -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=MIPS
+// RUN: %clang_cc1 -triple spir64 -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=SPIR64
+// RUN:

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

2020-05-05 Thread Erich Keane via Phabricator via cfe-commits

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



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4938
+if (EIT->getNumBits() > 128)
+  return getNaturalAlignIndirect(Ty, /*ByVal=*/true);
+

rjmccall wrote:
> rjmccall wrote:
> > erichkeane wrote:
> > > rjmccall wrote:
> > > > Does this need to consider the aggregate-as-array logic below?
> > > I'm not sure what you mean by this?  Are you suggesting I could/should 
> > > pass this as an array instead of indirectly?
> > My interpretation is that in general you're lowering large a `_ExtInt` like 
> > a struct with a bunch of integer members in it.  My understanding is that, 
> > for this target, that would make it a homogeneous aggregate eligible for 
> > the special treatment given to certain aggregate types below.  I don't know 
> > what that corresponds to at the code-emission level.
> This seems to work, although it's unfortunate that it duplicates some of the 
> existing behavior.  Do we also need to modify `isHomogeneousAggregate` to 
> consider `ExtInts`?  And if we do, does that avoid any of the duplicate logic 
> here?
I think doing that would result in altering a number of other targets as well 
(thats not a PPC specific function I think?). 

While there IS a little duplication, it is just the 5 lines that calculates the 
array and coerces it. Otherwise the logic is just slightly different enough I 
don't think it would save much.

That said, perhaps there is value in extracting those 5 lines or so into a 
function.  I'll give that a shot.


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

https://reviews.llvm.org/D79118



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

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

2020-05-05 Thread John McCall via Phabricator via cfe-commits

rjmccall added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4938
+if (EIT->getNumBits() > 128)
+  return getNaturalAlignIndirect(Ty, /*ByVal=*/true);
+

rjmccall wrote:
> erichkeane wrote:
> > rjmccall wrote:
> > > Does this need to consider the aggregate-as-array logic below?
> > I'm not sure what you mean by this?  Are you suggesting I could/should pass 
> > this as an array instead of indirectly?
> My interpretation is that in general you're lowering large a `_ExtInt` like a 
> struct with a bunch of integer members in it.  My understanding is that, for 
> this target, that would make it a homogeneous aggregate eligible for the 
> special treatment given to certain aggregate types below.  I don't know what 
> that corresponds to at the code-emission level.
This seems to work, although it's unfortunate that it duplicates some of the 
existing behavior.  Do we also need to modify `isHomogeneousAggregate` to 
consider `ExtInts`?  And if we do, does that avoid any of the duplicate logic 
here?



Comment at: clang/lib/CodeGen/TargetInfo.cpp:6658
 /// Checks if the type is unsupported directly by the current target.
 static bool isUnsupportedType(ASTContext , QualType T) {
   if (!Context.getTargetInfo().hasFloat16Type() && T->isFloat16Type())

erichkeane wrote:
> rjmccall wrote:
> > Ugh.  Please go ahead and rename this to make it clear that it's an NVPTX 
> > helper function.
> :)  Moved to a member function.  Also, coerceToIntArrayWithLimit made sense 
> to do so as well, since it was the caller of this.
Thanks.


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

https://reviews.llvm.org/D79118



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

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

2020-05-05 Thread Erich Keane via Phabricator via cfe-commits

erichkeane updated this revision to Diff 261922.
erichkeane marked 2 inline comments as done.
erichkeane added a comment.

For PPC64, better emulate structs by passing _ExtInt as an array.


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

https://reviews.llvm.org/D79118

Files:
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Basic/Targets/ARC.h
  clang/lib/Basic/Targets/ARM.h
  clang/lib/Basic/Targets/Hexagon.h
  clang/lib/Basic/Targets/Lanai.h
  clang/lib/Basic/Targets/Mips.h
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/Basic/Targets/PNaCl.h
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/Basic/Targets/SPIR.h
  clang/lib/Basic/Targets/Sparc.h
  clang/lib/Basic/Targets/SystemZ.h
  clang/lib/Basic/Targets/WebAssembly.h
  clang/lib/Basic/Targets/XCore.h
  clang/lib/CodeGen/ABIInfo.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/ext-int-cc.c
  clang/test/CodeGen/ext-int-sanitizer.cpp
  clang/test/CodeGenCXX/ext-int.cpp
  clang/test/Sema/ext-int-not-supported.c

Index: clang/test/Sema/ext-int-not-supported.c
===
--- clang/test/Sema/ext-int-not-supported.c
+++ /dev/null
@@ -1,5 +0,0 @@
-// RUN: %clang_cc1 -triple armv7 -fsyntax-only -verify %s
-
-void foo() {
-  _ExtInt(33) a; // expected-error{{_ExtInt is not supported on this target}}
-}
Index: clang/test/CodeGenCXX/ext-int.cpp
===
--- clang/test/CodeGenCXX/ext-int.cpp
+++ clang/test/CodeGenCXX/ext-int.cpp
@@ -98,7 +98,7 @@
 };
 
 void UnderlyingTypeUsage(AsEnumUnderlyingType Param) {
-  // LIN: define void @_Z19UnderlyingTypeUsage20AsEnumUnderlyingType(i16 %
+  // LIN: define void @_Z19UnderlyingTypeUsage20AsEnumUnderlyingType(i9 signext %
   // WIN: define dso_local void @"?UnderlyingTypeUsage@@YAXW4AsEnumUnderlyingType@@@Z"(i9 %
   AsEnumUnderlyingType Var;
   // CHECK: alloca i9, align 2
Index: clang/test/CodeGen/ext-int-sanitizer.cpp
===
--- clang/test/CodeGen/ext-int-sanitizer.cpp
+++ clang/test/CodeGen/ext-int-sanitizer.cpp
@@ -145,8 +145,7 @@
 
   // Also triggers signed integer overflow.
   E / E;
-  // CHECK: %[[E1LOAD:.+]] = load i11
-  // CHECK: store i11 %[[E1LOAD]], i11* %[[EADDR:.+]]
+  // CHECK: %[[EADDR:.+]] = alloca i11
   // CHECK: %[[E:.+]] = load i11, i11* %[[EADDR]]
   // CHECK: %[[E2:.+]] = load i11, i11* %[[EADDR]]
   // CHECK: %[[NEZERO:.+]] = icmp ne i11 %[[E2]], 0
@@ -163,8 +162,7 @@
 // CHECK: define void @_Z6ShiftsU7_ExtIntILi9EEi
 void Shifts(_ExtInt(9) E) {
   E >> E;
-  // CHECK: %[[E1LOAD:.+]] = load i9, i9*
-  // CHECK: store i9 %[[E1LOAD]], i9* %[[EADDR:.+]]
+  // CHECK: %[[EADDR:.+]] = alloca i9
   // CHECK: %[[LHSE:.+]] = load i9, i9* %[[EADDR]]
   // CHECK: %[[RHSE:.+]] = load i9, i9* %[[EADDR]]
   // CHECK: %[[CMP:.+]] = icmp ule i9 %[[RHSE]], 8
@@ -227,8 +225,7 @@
  unsigned _ExtInt(23) SmallE,
  unsigned _ExtInt(35) BigE) {
   u = SmallE + SmallE;
-  // CHECK: %[[LOADBIGGESTE2:.+]] = load i23
-  // CHECK: store i23 %[[LOADBIGGESTE2]], i23* %[[BIGGESTEADDR:.+]]
+  // CHECK: %[[BIGGESTEADDR:.+]] = alloca i23
   // CHECK: %[[LOADE1:.+]] = load i23, i23* %[[BIGGESTEADDR]]
   // CHECK: %[[LOADE2:.+]] = load i23, i23* %[[BIGGESTEADDR]]
   // CHECK: %[[OFCALL:.+]] = call { i23, i1 } @llvm.uadd.with.overflow.i23(i23 %[[LOADE1]], i23 %[[LOADE2]])
Index: clang/test/CodeGen/ext-int-cc.c
===
--- clang/test/CodeGen/ext-int-cc.c
+++ clang/test/CodeGen/ext-int-cc.c
@@ -2,6 +2,32 @@
 // RUN: %clang_cc1 -triple x86_64-windows-pc -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=WIN64
 // RUN: %clang_cc1 -triple i386-gnu-linux -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=LIN32
 // RUN: %clang_cc1 -triple i386-windows-pc -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=WIN32
+// RUN: %clang_cc1 -triple le32-nacl -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=NACL
+// RUN: %clang_cc1 -triple nvptx64 -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=NVPTX64
+// RUN: %clang_cc1 -triple nvptx -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=NVPTX
+// RUN: %clang_cc1 -triple sparcv9 -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=SPARCV9
+// RUN: %clang_cc1 -triple sparc -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=SPARC
+// RUN: %clang_cc1 -triple mips64 -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=MIPS64
+// RUN: %clang_cc1 -triple mips -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=MIPS
+// RUN: %clang_cc1 -triple spir64 -O3 -disable-llvm-passes -emit-llvm -o -

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

2020-05-04 Thread Erich Keane via Phabricator via cfe-commits

erichkeane updated this revision to Diff 261820.
erichkeane marked 5 inline comments as done.
Herald added a reviewer: jdoerfert.

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

https://reviews.llvm.org/D79118

Files:
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Basic/Targets/ARC.h
  clang/lib/Basic/Targets/ARM.h
  clang/lib/Basic/Targets/Hexagon.h
  clang/lib/Basic/Targets/Lanai.h
  clang/lib/Basic/Targets/Mips.h
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/Basic/Targets/PNaCl.h
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/Basic/Targets/SPIR.h
  clang/lib/Basic/Targets/Sparc.h
  clang/lib/Basic/Targets/SystemZ.h
  clang/lib/Basic/Targets/WebAssembly.h
  clang/lib/Basic/Targets/XCore.h
  clang/lib/CodeGen/ABIInfo.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/ext-int-cc.c
  clang/test/CodeGen/ext-int-sanitizer.cpp
  clang/test/CodeGenCXX/ext-int.cpp
  clang/test/Sema/ext-int-not-supported.c

Index: clang/test/Sema/ext-int-not-supported.c
===
--- clang/test/Sema/ext-int-not-supported.c
+++ /dev/null
@@ -1,5 +0,0 @@
-// RUN: %clang_cc1 -triple armv7 -fsyntax-only -verify %s
-
-void foo() {
-  _ExtInt(33) a; // expected-error{{_ExtInt is not supported on this target}}
-}
Index: clang/test/CodeGenCXX/ext-int.cpp
===
--- clang/test/CodeGenCXX/ext-int.cpp
+++ clang/test/CodeGenCXX/ext-int.cpp
@@ -98,7 +98,7 @@
 };
 
 void UnderlyingTypeUsage(AsEnumUnderlyingType Param) {
-  // LIN: define void @_Z19UnderlyingTypeUsage20AsEnumUnderlyingType(i16 %
+  // LIN: define void @_Z19UnderlyingTypeUsage20AsEnumUnderlyingType(i9 signext %
   // WIN: define dso_local void @"?UnderlyingTypeUsage@@YAXW4AsEnumUnderlyingType@@@Z"(i9 %
   AsEnumUnderlyingType Var;
   // CHECK: alloca i9, align 2
Index: clang/test/CodeGen/ext-int-sanitizer.cpp
===
--- clang/test/CodeGen/ext-int-sanitizer.cpp
+++ clang/test/CodeGen/ext-int-sanitizer.cpp
@@ -145,8 +145,7 @@
 
   // Also triggers signed integer overflow.
   E / E;
-  // CHECK: %[[E1LOAD:.+]] = load i11
-  // CHECK: store i11 %[[E1LOAD]], i11* %[[EADDR:.+]]
+  // CHECK: %[[EADDR:.+]] = alloca i11
   // CHECK: %[[E:.+]] = load i11, i11* %[[EADDR]]
   // CHECK: %[[E2:.+]] = load i11, i11* %[[EADDR]]
   // CHECK: %[[NEZERO:.+]] = icmp ne i11 %[[E2]], 0
@@ -163,8 +162,7 @@
 // CHECK: define void @_Z6ShiftsU7_ExtIntILi9EEi
 void Shifts(_ExtInt(9) E) {
   E >> E;
-  // CHECK: %[[E1LOAD:.+]] = load i9, i9*
-  // CHECK: store i9 %[[E1LOAD]], i9* %[[EADDR:.+]]
+  // CHECK: %[[EADDR:.+]] = alloca i9
   // CHECK: %[[LHSE:.+]] = load i9, i9* %[[EADDR]]
   // CHECK: %[[RHSE:.+]] = load i9, i9* %[[EADDR]]
   // CHECK: %[[CMP:.+]] = icmp ule i9 %[[RHSE]], 8
@@ -227,8 +225,7 @@
  unsigned _ExtInt(23) SmallE,
  unsigned _ExtInt(35) BigE) {
   u = SmallE + SmallE;
-  // CHECK: %[[LOADBIGGESTE2:.+]] = load i23
-  // CHECK: store i23 %[[LOADBIGGESTE2]], i23* %[[BIGGESTEADDR:.+]]
+  // CHECK: %[[BIGGESTEADDR:.+]] = alloca i23
   // CHECK: %[[LOADE1:.+]] = load i23, i23* %[[BIGGESTEADDR]]
   // CHECK: %[[LOADE2:.+]] = load i23, i23* %[[BIGGESTEADDR]]
   // CHECK: %[[OFCALL:.+]] = call { i23, i1 } @llvm.uadd.with.overflow.i23(i23 %[[LOADE1]], i23 %[[LOADE2]])
Index: clang/test/CodeGen/ext-int-cc.c
===
--- clang/test/CodeGen/ext-int-cc.c
+++ clang/test/CodeGen/ext-int-cc.c
@@ -2,6 +2,32 @@
 // RUN: %clang_cc1 -triple x86_64-windows-pc -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=WIN64
 // RUN: %clang_cc1 -triple i386-gnu-linux -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=LIN32
 // RUN: %clang_cc1 -triple i386-windows-pc -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=WIN32
+// RUN: %clang_cc1 -triple le32-nacl -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=NACL
+// RUN: %clang_cc1 -triple nvptx64 -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=NVPTX64
+// RUN: %clang_cc1 -triple nvptx -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=NVPTX
+// RUN: %clang_cc1 -triple sparcv9 -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=SPARCV9
+// RUN: %clang_cc1 -triple sparc -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=SPARC
+// RUN: %clang_cc1 -triple mips64 -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=MIPS64
+// RUN: %clang_cc1 -triple mips -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=MIPS
+// RUN: %clang_cc1 -triple spir64 -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=SPIR64
+// RUN:

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

2020-05-04 Thread John McCall via Phabricator via cfe-commits

rjmccall added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4938
+if (EIT->getNumBits() > 128)
+  return getNaturalAlignIndirect(Ty, /*ByVal=*/true);
+

erichkeane wrote:
> rjmccall wrote:
> > Does this need to consider the aggregate-as-array logic below?
> I'm not sure what you mean by this?  Are you suggesting I could/should pass 
> this as an array instead of indirectly?
My interpretation is that in general you're lowering large a `_ExtInt` like a 
struct with a bunch of integer members in it.  My understanding is that, for 
this target, that would make it a homogeneous aggregate eligible for the 
special treatment given to certain aggregate types below.  I don't know what 
that corresponds to at the code-emission level.


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

https://reviews.llvm.org/D79118



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

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

2020-05-04 Thread Erich Keane via Phabricator via cfe-commits

erichkeane marked 15 inline comments as done.
erichkeane added a comment.

Thanks for the review @rjmccall .  Updated patch coming momentarily.  I audited 
my 'byval' use as well.  I still have 1 open, the 'array' comment which I 
wasn't clear what you meant.




Comment at: clang/lib/CodeGen/TargetInfo.cpp:719
+ !getContext().getTargetInfo().hasInt128Type()))
+  return getNaturalAlignIndirect(Ty);
+

rjmccall wrote:
> It's very weird for 64 and 128 to be showing up as constants in the default 
> ABI rule.
Good point.  I rewrote this in terms of the Context.LongLongTy and 
Context.Int128Ty.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:1537
+  return (isPromotableIntegerType(RetTy) ? ABIArgInfo::getExtend(RetTy)
+ : ABIArgInfo::getDirect());
 }

rjmccall wrote:
> Won't this treat literally all ExtInt types as either extend or direct?
Yes it does, and I justified doing it that way at one point, but I cannot 
remember why.

NVPTX seems to pass everything by value in return anyway(including large 
structures!), so I'm doing that one on purpose.

SparcV9 actually has a 256 bit limit, so I added a test to explicitly check it.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4938
+if (EIT->getNumBits() > 128)
+  return getNaturalAlignIndirect(Ty, /*ByVal=*/true);
+

rjmccall wrote:
> Does this need to consider the aggregate-as-array logic below?
I'm not sure what you mean by this?  Are you suggesting I could/should pass 
this as an array instead of indirectly?



Comment at: clang/lib/CodeGen/TargetInfo.cpp:5014
+if (EIT->getNumBits() > 128)
+  return getNaturalAlignIndirect(RetTy, /*ByVal=*/true);
+

rjmccall wrote:
> `byval` is not legal on return values.
Ah, neat! I didn't realize that.  Thanks.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:6658
 /// Checks if the type is unsupported directly by the current target.
 static bool isUnsupportedType(ASTContext , QualType T) {
   if (!Context.getTargetInfo().hasFloat16Type() && T->isFloat16Type())

rjmccall wrote:
> Ugh.  Please go ahead and rename this to make it clear that it's an NVPTX 
> helper function.
:)  Moved to a member function.  Also, coerceToIntArrayWithLimit made sense to 
do so as well, since it was the caller of this.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:8988
 
-  uint64_t Size = getContext().getTypeSize(Ty);
+  uint64_t Size = getContext().getIntWidth(Ty);
 

rjmccall wrote:
> Why this change?
So that line 9000 will 'extend' 63 bit sizes.  getTypeSize rounds to power of 
2, getIntWidth doesn't. 

As it was unclear, I've instead added this as an explicit case below.


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

https://reviews.llvm.org/D79118



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

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

2020-05-02 Thread John McCall via Phabricator via cfe-commits

rjmccall added a comment.

A little cavalier with `byval`; I gave it a thorough audit, but you might want 
your own pass.




Comment at: clang/lib/CodeGen/ABIInfo.h:107
+// only difference is that this considers _ExtInt as well.
+bool isPromotableIntegerType(QualType Ty) const;
+

Maybe add a "ForABI" suffix here so that it's clearer that this is a 
specialized use-case.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:719
+ !getContext().getTargetInfo().hasInt128Type()))
+  return getNaturalAlignIndirect(Ty);
+

It's very weird for 64 and 128 to be showing up as constants in the default ABI 
rule.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:1537
+  return (isPromotableIntegerType(RetTy) ? ABIArgInfo::getExtend(RetTy)
+ : ABIArgInfo::getDirect());
 }

Won't this treat literally all ExtInt types as either extend or direct?



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4938
+if (EIT->getNumBits() > 128)
+  return getNaturalAlignIndirect(Ty, /*ByVal=*/true);
+

Does this need to consider the aggregate-as-array logic below?



Comment at: clang/lib/CodeGen/TargetInfo.cpp:5014
+if (EIT->getNumBits() > 128)
+  return getNaturalAlignIndirect(RetTy, /*ByVal=*/true);
+

`byval` is not legal on return values.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:6339
+  if (EIT->getNumBits() > 64)
+return getNaturalAlignIndirect(RetTy, /*ByVal=*/true);
+

`byval` is not legal on return types.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:6658
 /// Checks if the type is unsupported directly by the current target.
 static bool isUnsupportedType(ASTContext , QualType T) {
   if (!Context.getTargetInfo().hasFloat16Type() && T->isFloat16Type())

Ugh.  Please go ahead and rename this to make it clear that it's an NVPTX 
helper function.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:7921
+if (Size > 64 && RetTy->isExtIntType())
+  return getNaturalAlignIndirect(RetTy, /*ByVal=*/true);
+

`byval` is not legal on a return.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:8988
 
-  uint64_t Size = getContext().getTypeSize(Ty);
+  uint64_t Size = getContext().getIntWidth(Ty);
 

Why this change?



Comment at: clang/lib/CodeGen/TargetInfo.cpp:10297
+   EIT->getNumBits() > 64))
+return getNaturalAlignIndirect(Ty, /*ByVal=*/true);
+}

Looks like this shouldn't be `byval`.


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

https://reviews.llvm.org/D79118



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

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

2020-05-01 Thread Erich Keane via Phabricator via cfe-commits

erichkeane updated this revision to Diff 261523.
erichkeane retitled this revision from "[WIP] _ExtInt calling convention Audit" 
to "Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs".
erichkeane edited the summary of this revision.
erichkeane added a subscriber: cfe-commits.
erichkeane added a comment.
Herald added subscribers: kerbowa, luismarques, apazos, sameer.abuasal, pzheng, 
lenary, Jim, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, 
atanasyan, edward-jones, zzheng, MaskRay, niosHD, sabuasal, johnrusso, rbar, 
asb, kbarton, jgravelle-google, sbc100, nhaehnle, jvesely, nemanjai, sdardis, 
jyknight, jholewinski.

@rjmccall

Alright, i finished the audit and finished all of the ABIs, as well as wrote 
tests for each.  I added a test for situations that I'd not previously covered 
as well.

Can you please review when you can/suggest others to do so?

Thanks!


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

https://reviews.llvm.org/D79118

Files:
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Basic/Targets/ARC.h
  clang/lib/Basic/Targets/ARM.h
  clang/lib/Basic/Targets/Hexagon.h
  clang/lib/Basic/Targets/Lanai.h
  clang/lib/Basic/Targets/Mips.h
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/Basic/Targets/PNaCl.h
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/Basic/Targets/SPIR.h
  clang/lib/Basic/Targets/Sparc.h
  clang/lib/Basic/Targets/SystemZ.h
  clang/lib/Basic/Targets/WebAssembly.h
  clang/lib/Basic/Targets/XCore.h
  clang/lib/CodeGen/ABIInfo.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/ext-int-cc.c
  clang/test/CodeGen/ext-int-sanitizer.cpp
  clang/test/CodeGenCXX/ext-int.cpp

Index: clang/test/CodeGenCXX/ext-int.cpp
===
--- clang/test/CodeGenCXX/ext-int.cpp
+++ clang/test/CodeGenCXX/ext-int.cpp
@@ -98,7 +98,7 @@
 };
 
 void UnderlyingTypeUsage(AsEnumUnderlyingType Param) {
-  // LIN: define void @_Z19UnderlyingTypeUsage20AsEnumUnderlyingType(i16 %
+  // LIN: define void @_Z19UnderlyingTypeUsage20AsEnumUnderlyingType(i9 signext %
   // WIN: define dso_local void @"?UnderlyingTypeUsage@@YAXW4AsEnumUnderlyingType@@@Z"(i9 %
   AsEnumUnderlyingType Var;
   // CHECK: alloca i9, align 2
Index: clang/test/CodeGen/ext-int-sanitizer.cpp
===
--- clang/test/CodeGen/ext-int-sanitizer.cpp
+++ clang/test/CodeGen/ext-int-sanitizer.cpp
@@ -145,8 +145,7 @@
 
   // Also triggers signed integer overflow.
   E / E;
-  // CHECK: %[[E1LOAD:.+]] = load i11
-  // CHECK: store i11 %[[E1LOAD]], i11* %[[EADDR:.+]]
+  // CHECK: %[[EADDR:.+]] = alloca i11
   // CHECK: %[[E:.+]] = load i11, i11* %[[EADDR]]
   // CHECK: %[[E2:.+]] = load i11, i11* %[[EADDR]]
   // CHECK: %[[NEZERO:.+]] = icmp ne i11 %[[E2]], 0
@@ -163,8 +162,7 @@
 // CHECK: define void @_Z6ShiftsU7_ExtIntILi9EEi
 void Shifts(_ExtInt(9) E) {
   E >> E;
-  // CHECK: %[[E1LOAD:.+]] = load i9, i9*
-  // CHECK: store i9 %[[E1LOAD]], i9* %[[EADDR:.+]]
+  // CHECK: %[[EADDR:.+]] = alloca i9
   // CHECK: %[[LHSE:.+]] = load i9, i9* %[[EADDR]]
   // CHECK: %[[RHSE:.+]] = load i9, i9* %[[EADDR]]
   // CHECK: %[[CMP:.+]] = icmp ule i9 %[[RHSE]], 8
@@ -227,8 +225,7 @@
  unsigned _ExtInt(23) SmallE,
  unsigned _ExtInt(35) BigE) {
   u = SmallE + SmallE;
-  // CHECK: %[[LOADBIGGESTE2:.+]] = load i23
-  // CHECK: store i23 %[[LOADBIGGESTE2]], i23* %[[BIGGESTEADDR:.+]]
+  // CHECK: %[[BIGGESTEADDR:.+]] = alloca i23
   // CHECK: %[[LOADE1:.+]] = load i23, i23* %[[BIGGESTEADDR]]
   // CHECK: %[[LOADE2:.+]] = load i23, i23* %[[BIGGESTEADDR]]
   // CHECK: %[[OFCALL:.+]] = call { i23, i1 } @llvm.uadd.with.overflow.i23(i23 %[[LOADE1]], i23 %[[LOADE2]])
Index: clang/test/CodeGen/ext-int-cc.c
===
--- clang/test/CodeGen/ext-int-cc.c
+++ clang/test/CodeGen/ext-int-cc.c
@@ -2,6 +2,32 @@
 // RUN: %clang_cc1 -triple x86_64-windows-pc -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=WIN64
 // RUN: %clang_cc1 -triple i386-gnu-linux -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=LIN32
 // RUN: %clang_cc1 -triple i386-windows-pc -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=WIN32
+// RUN: %clang_cc1 -triple le32-nacl -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=NACL
+// RUN: %clang_cc1 -triple nvptx64 -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=NVPTX64
+// RUN: %clang_cc1 -triple nvptx -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=NVPTX
+// RUN: %clang_cc1 -triple sparcv9 -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=SPARCV9
+// RUN: %clang_cc1 -triple sparc -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

2020-05-01 Thread Erich Keane via Phabricator via cfe-commits

erichkeane updated this revision to Diff 261524.
erichkeane edited the summary of this revision.
erichkeane added a comment.
Herald added a subscriber: wuzish.

Remove test for which there is no longer a target where extint isn't supported 
(thus I cannot spell the condition).  Also, clang-formatted the previous 
revision.


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

https://reviews.llvm.org/D79118

Files:
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Basic/Targets/ARC.h
  clang/lib/Basic/Targets/ARM.h
  clang/lib/Basic/Targets/Hexagon.h
  clang/lib/Basic/Targets/Lanai.h
  clang/lib/Basic/Targets/Mips.h
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/Basic/Targets/PNaCl.h
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/Basic/Targets/SPIR.h
  clang/lib/Basic/Targets/Sparc.h
  clang/lib/Basic/Targets/SystemZ.h
  clang/lib/Basic/Targets/WebAssembly.h
  clang/lib/Basic/Targets/XCore.h
  clang/lib/CodeGen/ABIInfo.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/ext-int-cc.c
  clang/test/CodeGen/ext-int-sanitizer.cpp
  clang/test/CodeGenCXX/ext-int.cpp
  clang/test/Sema/ext-int-not-supported.c

Index: clang/test/Sema/ext-int-not-supported.c
===
--- clang/test/Sema/ext-int-not-supported.c
+++ /dev/null
@@ -1,5 +0,0 @@
-// RUN: %clang_cc1 -triple armv7 -fsyntax-only -verify %s
-
-void foo() {
-  _ExtInt(33) a; // expected-error{{_ExtInt is not supported on this target}}
-}
Index: clang/test/CodeGenCXX/ext-int.cpp
===
--- clang/test/CodeGenCXX/ext-int.cpp
+++ clang/test/CodeGenCXX/ext-int.cpp
@@ -98,7 +98,7 @@
 };
 
 void UnderlyingTypeUsage(AsEnumUnderlyingType Param) {
-  // LIN: define void @_Z19UnderlyingTypeUsage20AsEnumUnderlyingType(i16 %
+  // LIN: define void @_Z19UnderlyingTypeUsage20AsEnumUnderlyingType(i9 signext %
   // WIN: define dso_local void @"?UnderlyingTypeUsage@@YAXW4AsEnumUnderlyingType@@@Z"(i9 %
   AsEnumUnderlyingType Var;
   // CHECK: alloca i9, align 2
Index: clang/test/CodeGen/ext-int-sanitizer.cpp
===
--- clang/test/CodeGen/ext-int-sanitizer.cpp
+++ clang/test/CodeGen/ext-int-sanitizer.cpp
@@ -145,8 +145,7 @@
 
   // Also triggers signed integer overflow.
   E / E;
-  // CHECK: %[[E1LOAD:.+]] = load i11
-  // CHECK: store i11 %[[E1LOAD]], i11* %[[EADDR:.+]]
+  // CHECK: %[[EADDR:.+]] = alloca i11
   // CHECK: %[[E:.+]] = load i11, i11* %[[EADDR]]
   // CHECK: %[[E2:.+]] = load i11, i11* %[[EADDR]]
   // CHECK: %[[NEZERO:.+]] = icmp ne i11 %[[E2]], 0
@@ -163,8 +162,7 @@
 // CHECK: define void @_Z6ShiftsU7_ExtIntILi9EEi
 void Shifts(_ExtInt(9) E) {
   E >> E;
-  // CHECK: %[[E1LOAD:.+]] = load i9, i9*
-  // CHECK: store i9 %[[E1LOAD]], i9* %[[EADDR:.+]]
+  // CHECK: %[[EADDR:.+]] = alloca i9
   // CHECK: %[[LHSE:.+]] = load i9, i9* %[[EADDR]]
   // CHECK: %[[RHSE:.+]] = load i9, i9* %[[EADDR]]
   // CHECK: %[[CMP:.+]] = icmp ule i9 %[[RHSE]], 8
@@ -227,8 +225,7 @@
  unsigned _ExtInt(23) SmallE,
  unsigned _ExtInt(35) BigE) {
   u = SmallE + SmallE;
-  // CHECK: %[[LOADBIGGESTE2:.+]] = load i23
-  // CHECK: store i23 %[[LOADBIGGESTE2]], i23* %[[BIGGESTEADDR:.+]]
+  // CHECK: %[[BIGGESTEADDR:.+]] = alloca i23
   // CHECK: %[[LOADE1:.+]] = load i23, i23* %[[BIGGESTEADDR]]
   // CHECK: %[[LOADE2:.+]] = load i23, i23* %[[BIGGESTEADDR]]
   // CHECK: %[[OFCALL:.+]] = call { i23, i1 } @llvm.uadd.with.overflow.i23(i23 %[[LOADE1]], i23 %[[LOADE2]])
Index: clang/test/CodeGen/ext-int-cc.c
===
--- clang/test/CodeGen/ext-int-cc.c
+++ clang/test/CodeGen/ext-int-cc.c
@@ -2,6 +2,32 @@
 // RUN: %clang_cc1 -triple x86_64-windows-pc -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=WIN64
 // RUN: %clang_cc1 -triple i386-gnu-linux -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=LIN32
 // RUN: %clang_cc1 -triple i386-windows-pc -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=WIN32
+// RUN: %clang_cc1 -triple le32-nacl -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=NACL
+// RUN: %clang_cc1 -triple nvptx64 -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=NVPTX64
+// RUN: %clang_cc1 -triple nvptx -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=NVPTX
+// RUN: %clang_cc1 -triple sparcv9 -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=SPARCV9
+// RUN: %clang_cc1 -triple sparc -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=SPARC
+// RUN: %clang_cc1 -triple mips64 -O3 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefixes=MIPS64
+// RUN: %clang_cc1 -triple mips -O3

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

17 matches

Site Navigation

Mail list logo

Footer information