[PATCH] D80323: [SVE] Eliminate calls to default-false VectorType::get() from Clang

2020-06-01 Thread Christopher Tetreault via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG796898172c48: [SVE] Eliminate calls to default-false 
VectorType::get() from Clang (authored by ctetreau).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80323

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/SwiftCallingConv.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/utils/TableGen/MveEmitter.cpp

Index: clang/utils/TableGen/MveEmitter.cpp
===
--- clang/utils/TableGen/MveEmitter.cpp
+++ clang/utils/TableGen/MveEmitter.cpp
@@ -300,7 +300,7 @@
 return Element->cNameBase() + "x" + utostr(Lanes);
   }
   std::string llvmName() const override {
-return "llvm::VectorType::get(" + Element->llvmName() + ", " +
+return "llvm::FixedVectorType::get(" + Element->llvmName() + ", " +
utostr(Lanes) + ")";
   }
 
@@ -354,7 +354,7 @@
 // explanation.
 unsigned ModifiedLanes = (Lanes == 2 ? 4 : Lanes);
 
-return "llvm::VectorType::get(Builder.getInt1Ty(), " +
+return "llvm::FixedVectorType::get(Builder.getInt1Ty(), " +
utostr(ModifiedLanes) + ")";
   }
 
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -1478,8 +1478,8 @@
   // registers and we need to make sure to pick a type the LLVM
   // backend will like.
   if (Size == 128)
-return ABIArgInfo::getDirect(llvm::VectorType::get(
-  llvm::Type::getInt64Ty(getVMContext()), 2));
+return ABIArgInfo::getDirect(llvm::FixedVectorType::get(
+llvm::Type::getInt64Ty(getVMContext()), 2));
 
   // Always return in register if it fits in a general purpose
   // register, or if it is 64 bits and has a single element.
@@ -3122,8 +3122,8 @@
 cast(IRType)->getElementType()->isIntegerTy(128)) {
   // Use a vXi64 vector.
   uint64_t Size = getContext().getTypeSize(Ty);
-  return llvm::VectorType::get(llvm::Type::getInt64Ty(getVMContext()),
-   Size / 64);
+  return llvm::FixedVectorType::get(llvm::Type::getInt64Ty(getVMContext()),
+Size / 64);
 }
 
 return IRType;
@@ -3138,8 +3138,8 @@
 
 
   // Return a LLVM IR vector type based on the size of 'Ty'.
-  return llvm::VectorType::get(llvm::Type::getDoubleTy(getVMContext()),
-   Size / 64);
+  return llvm::FixedVectorType::get(llvm::Type::getDoubleTy(getVMContext()),
+Size / 64);
 }
 
 /// BitsContainNoUserData - Return true if the specified [start,end) bit range
@@ -3273,7 +3273,8 @@
   // case.
   if (ContainsFloatAtOffset(IRType, IROffset, getDataLayout()) &&
   ContainsFloatAtOffset(IRType, IROffset+4, getDataLayout()))
-return llvm::VectorType::get(llvm::Type::getFloatTy(getVMContext()), 2);
+return llvm::FixedVectorType::get(llvm::Type::getFloatTy(getVMContext()),
+  2);
 
   return llvm::Type::getDoubleTy(getVMContext());
 }
@@ -4140,8 +4141,8 @@
 
   // Mingw64 GCC returns i128 in XMM0. Coerce to v2i64 to handle that.
   // Clang matches them for compatibility.
-  return ABIArgInfo::getDirect(
-  llvm::VectorType::get(llvm::Type::getInt64Ty(getVMContext()), 2));
+  return ABIArgInfo::getDirect(llvm::FixedVectorType::get(
+  llvm::Type::getInt64Ty(getVMContext()), 2));
 
 default:
   break;
@@ -5478,13 +5479,13 @@
   return ABIArgInfo::getDirect(ResType);
 }
 if (Size == 64) {
-  llvm::Type *ResType =
-  llvm::VectorType::get(llvm::Type::getInt32Ty(getVMContext()), 2);
+  auto *ResType =
+  llvm::FixedVectorType::get(llvm::Type::getInt32Ty(getVMContext()), 2);
   return ABIArgInfo::getDirect(ResType);
 }
 if (Size == 128) {
-  llvm::Type *ResType =
-  llvm::VectorType::get(llvm::Type::getInt32Ty(getVMContext()), 4);
+  auto *ResType =
+  llvm::FixedVectorType::get(llvm::Type::getInt32Ty(getVMContext()), 4);
   return ABIArgInfo::getDirect(ResType);
 }
 return getNaturalAlignIndirect(Ty, /*ByVal=*/false);
@@ -6209,7 +6210,7 @@
 return ABIArgInfo::getDirect(ResType);
   }
   if (Size == 64 || Size == 128) {
-llvm::Type *ResType = llvm::VectorType::get(
+auto *ResType = llvm::FixedVectorType::get(
 llvm::Type::getInt32Ty(getVMContext()), Size / 32);
 return ABIArgInfo::getDirect(ResType);
   }
@@ -6225,7 +6226,7 @@
 // FP16 vectors should be converted to integer vectors
 if (!getTarget().hasLegalHalfType() && containsAnyFP16Vectors(Ty)) {
   uint64_t Size =

[PATCH] D80323: [SVE] Eliminate calls to default-false VectorType::get() from Clang

2020-05-29 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.

Yeah, LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80323



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

[PATCH] D80323: [SVE] Eliminate calls to default-false VectorType::get() from Clang

2020-05-29 Thread Christopher Tetreault via Phabricator via cfe-commits 
ctetreau added a comment.

@rjmccall Given the outcome of the call yesterday, may I merge this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80323



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

[PATCH] D80323: [SVE] Eliminate calls to default-false VectorType::get() from Clang

2020-05-28 Thread Christopher Tetreault via Phabricator via cfe-commits 
ctetreau updated this revision to Diff 267002.
ctetreau added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80323

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/SwiftCallingConv.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/utils/TableGen/MveEmitter.cpp

Index: clang/utils/TableGen/MveEmitter.cpp
===
--- clang/utils/TableGen/MveEmitter.cpp
+++ clang/utils/TableGen/MveEmitter.cpp
@@ -300,7 +300,7 @@
 return Element->cNameBase() + "x" + utostr(Lanes);
   }
   std::string llvmName() const override {
-return "llvm::VectorType::get(" + Element->llvmName() + ", " +
+return "llvm::FixedVectorType::get(" + Element->llvmName() + ", " +
utostr(Lanes) + ")";
   }
 
@@ -354,7 +354,7 @@
 // explanation.
 unsigned ModifiedLanes = (Lanes == 2 ? 4 : Lanes);
 
-return "llvm::VectorType::get(Builder.getInt1Ty(), " +
+return "llvm::FixedVectorType::get(Builder.getInt1Ty(), " +
utostr(ModifiedLanes) + ")";
   }
 
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -1478,8 +1478,8 @@
   // registers and we need to make sure to pick a type the LLVM
   // backend will like.
   if (Size == 128)
-return ABIArgInfo::getDirect(llvm::VectorType::get(
-  llvm::Type::getInt64Ty(getVMContext()), 2));
+return ABIArgInfo::getDirect(llvm::FixedVectorType::get(
+llvm::Type::getInt64Ty(getVMContext()), 2));
 
   // Always return in register if it fits in a general purpose
   // register, or if it is 64 bits and has a single element.
@@ -3122,8 +3122,8 @@
 cast(IRType)->getElementType()->isIntegerTy(128)) {
   // Use a vXi64 vector.
   uint64_t Size = getContext().getTypeSize(Ty);
-  return llvm::VectorType::get(llvm::Type::getInt64Ty(getVMContext()),
-   Size / 64);
+  return llvm::FixedVectorType::get(llvm::Type::getInt64Ty(getVMContext()),
+Size / 64);
 }
 
 return IRType;
@@ -3138,8 +3138,8 @@
 
 
   // Return a LLVM IR vector type based on the size of 'Ty'.
-  return llvm::VectorType::get(llvm::Type::getDoubleTy(getVMContext()),
-   Size / 64);
+  return llvm::FixedVectorType::get(llvm::Type::getDoubleTy(getVMContext()),
+Size / 64);
 }
 
 /// BitsContainNoUserData - Return true if the specified [start,end) bit range
@@ -3273,7 +3273,8 @@
   // case.
   if (ContainsFloatAtOffset(IRType, IROffset, getDataLayout()) &&
   ContainsFloatAtOffset(IRType, IROffset+4, getDataLayout()))
-return llvm::VectorType::get(llvm::Type::getFloatTy(getVMContext()), 2);
+return llvm::FixedVectorType::get(llvm::Type::getFloatTy(getVMContext()),
+  2);
 
   return llvm::Type::getDoubleTy(getVMContext());
 }
@@ -4140,8 +4141,8 @@
 
   // Mingw64 GCC returns i128 in XMM0. Coerce to v2i64 to handle that.
   // Clang matches them for compatibility.
-  return ABIArgInfo::getDirect(
-  llvm::VectorType::get(llvm::Type::getInt64Ty(getVMContext()), 2));
+  return ABIArgInfo::getDirect(llvm::FixedVectorType::get(
+  llvm::Type::getInt64Ty(getVMContext()), 2));
 
 default:
   break;
@@ -5478,13 +5479,13 @@
   return ABIArgInfo::getDirect(ResType);
 }
 if (Size == 64) {
-  llvm::Type *ResType =
-  llvm::VectorType::get(llvm::Type::getInt32Ty(getVMContext()), 2);
+  auto *ResType =
+  llvm::FixedVectorType::get(llvm::Type::getInt32Ty(getVMContext()), 2);
   return ABIArgInfo::getDirect(ResType);
 }
 if (Size == 128) {
-  llvm::Type *ResType =
-  llvm::VectorType::get(llvm::Type::getInt32Ty(getVMContext()), 4);
+  auto *ResType =
+  llvm::FixedVectorType::get(llvm::Type::getInt32Ty(getVMContext()), 4);
   return ABIArgInfo::getDirect(ResType);
 }
 return getNaturalAlignIndirect(Ty, /*ByVal=*/false);
@@ -6209,7 +6210,7 @@
 return ABIArgInfo::getDirect(ResType);
   }
   if (Size == 64 || Size == 128) {
-llvm::Type *ResType = llvm::VectorType::get(
+auto *ResType = llvm::FixedVectorType::get(
 llvm::Type::getInt32Ty(getVMContext()), Size / 32);
 return ABIArgInfo::getDirect(ResType);
   }
@@ -6225,7 +6226,7 @@
 // FP16 vectors should be converted to integer vectors
 if (!getTarget().hasLegalHalfType() && containsAnyFP16Vectors(Ty)) {
   uint64_t Size = getContext().getTypeSize(VT);
-  llvm::Type *NewVecTy = llvm::VectorType::get(
+  auto *NewVecTy =

[PATCH] D80323: [SVE] Eliminate calls to default-false VectorType::get() from Clang

2020-05-21 Thread John McCall via Phabricator via cfe-commits 
rjmccall requested changes to this revision.
rjmccall added a comment.
This revision now requires changes to proceed.

I've responded to the llvmdev thread, which I missed before.  I would like us 
to hold off on this line or work until we come to a resolution there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80323



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

[PATCH] D80323: [SVE] Eliminate calls to default-false VectorType::get() from Clang

2020-05-21 Thread John McCall via Phabricator via cfe-commits 
rjmccall added a comment.

In D80323#2049374 , @ctetreau wrote:

> In D80323#2048457 , @rjmccall wrote:
>
> > I'm sympathetic to wanting to get rid of the boolean flag, but this is a 
> > really invasive change for pretty minimal benefit.  Why not leave 
> > `VectorType::get` as meaning a non-scalable vector type and come up with a 
> > different method name to get a scalable vector?
>
>
> As it stands today, if you call VectorType::get(Ty, N, false), you will get 
> an instance of FixedVectorType, via a base VectorType pointer. Currently, you 
> can call getNumElements() on a base VectorType and it will work, but the 
> ultimate endgame is that getNumElements is going to be removed from base 
> VectorType. This change makes it so that you don't have to cast your 
> VectorType object to FixedVectorType everywhere.
>
> The overall architecture is that there is a derived type for each sort of 
> vector. VectorType is the base class, and the two derived vector types are 
> FixedVectorType and ScalableVectorType. I suppose I could have named them 
> something like BaseVectorType, VectorType(actually fixed width vector type), 
> and ScalableVectorType, but I feel that this is a worse solution because it's 
> not obvious by the name what a "VectorType" is. Additionally, naming the 
> types this would have broken all code that previously worked for scalable 
> vectors. It's not obvious to me which naming scheme would have resulted in 
> less changes (changes to rename fixed vectors to FixedVectorType vs changes 
> needed to rename all generic code to BaseVectorType and to fix code for 
> scalable vectors), but I think the consistency in the naming scheme justifies 
> the path I chose.


Yeah, I understand what you're trying to do.  If I had to guess, I'd say the 
vast majority of existing frontend, optimizer, and backend code does *not* work 
with scalable vectors as a free generalization.  The code that *does* work with 
them needs to at least be audited, and the clearest way of marking that you've 
done that audit would be to change the types.  So this is breaking a ton of 
code, including a lot that's not in-tree and which you are therefore not on the 
hook to update (and it's particularly likely that there's a lot of vector code 
out-of-tree), as well as setting up the "audit polarity" exactly backwards from 
what it should be — all just so that you can use the name `VectorType` for the 
common base type, which is really not much of a win vs. `VectorBaseType` or 
`AnyVectorType` or `AbstractVectorType` or some similar.

The fact is that people come up with ways to generalize the representation all 
the time, and it's great for LLVM to take those.  Sometimes generalizations are 
necessarily disruptive because they're really pointing out a common conflation 
that's dangerous in some way, but in this case it's purely "additive" and 
there's really no good reason that any of the existing code that's happily 
assuming fixed-width vectors should have to change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80323



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

[PATCH] D80323: [SVE] Eliminate calls to default-false VectorType::get() from Clang

2020-05-21 Thread Christopher Tetreault via Phabricator via cfe-commits 
ctetreau updated this revision to Diff 265542.
ctetreau added a comment.

address code review issues


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80323

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/SwiftCallingConv.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/utils/TableGen/MveEmitter.cpp

Index: clang/utils/TableGen/MveEmitter.cpp
===
--- clang/utils/TableGen/MveEmitter.cpp
+++ clang/utils/TableGen/MveEmitter.cpp
@@ -300,7 +300,7 @@
 return Element->cNameBase() + "x" + utostr(Lanes);
   }
   std::string llvmName() const override {
-return "llvm::VectorType::get(" + Element->llvmName() + ", " +
+return "llvm::FixedVectorType::get(" + Element->llvmName() + ", " +
utostr(Lanes) + ")";
   }
 
@@ -354,7 +354,7 @@
 // explanation.
 unsigned ModifiedLanes = (Lanes == 2 ? 4 : Lanes);
 
-return "llvm::VectorType::get(Builder.getInt1Ty(), " +
+return "llvm::FixedVectorType::get(Builder.getInt1Ty(), " +
utostr(ModifiedLanes) + ")";
   }
 
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -1478,8 +1478,8 @@
   // registers and we need to make sure to pick a type the LLVM
   // backend will like.
   if (Size == 128)
-return ABIArgInfo::getDirect(llvm::VectorType::get(
-  llvm::Type::getInt64Ty(getVMContext()), 2));
+return ABIArgInfo::getDirect(llvm::FixedVectorType::get(
+llvm::Type::getInt64Ty(getVMContext()), 2));
 
   // Always return in register if it fits in a general purpose
   // register, or if it is 64 bits and has a single element.
@@ -3122,8 +3122,8 @@
 cast(IRType)->getElementType()->isIntegerTy(128)) {
   // Use a vXi64 vector.
   uint64_t Size = getContext().getTypeSize(Ty);
-  return llvm::VectorType::get(llvm::Type::getInt64Ty(getVMContext()),
-   Size / 64);
+  return llvm::FixedVectorType::get(llvm::Type::getInt64Ty(getVMContext()),
+Size / 64);
 }
 
 return IRType;
@@ -3138,8 +3138,8 @@
 
 
   // Return a LLVM IR vector type based on the size of 'Ty'.
-  return llvm::VectorType::get(llvm::Type::getDoubleTy(getVMContext()),
-   Size / 64);
+  return llvm::FixedVectorType::get(llvm::Type::getDoubleTy(getVMContext()),
+Size / 64);
 }
 
 /// BitsContainNoUserData - Return true if the specified [start,end) bit range
@@ -3273,7 +3273,8 @@
   // case.
   if (ContainsFloatAtOffset(IRType, IROffset, getDataLayout()) &&
   ContainsFloatAtOffset(IRType, IROffset+4, getDataLayout()))
-return llvm::VectorType::get(llvm::Type::getFloatTy(getVMContext()), 2);
+return llvm::FixedVectorType::get(llvm::Type::getFloatTy(getVMContext()),
+  2);
 
   return llvm::Type::getDoubleTy(getVMContext());
 }
@@ -4140,8 +4141,8 @@
 
   // Mingw64 GCC returns i128 in XMM0. Coerce to v2i64 to handle that.
   // Clang matches them for compatibility.
-  return ABIArgInfo::getDirect(
-  llvm::VectorType::get(llvm::Type::getInt64Ty(getVMContext()), 2));
+  return ABIArgInfo::getDirect(llvm::FixedVectorType::get(
+  llvm::Type::getInt64Ty(getVMContext()), 2));
 
 default:
   break;
@@ -5478,13 +5479,13 @@
   return ABIArgInfo::getDirect(ResType);
 }
 if (Size == 64) {
-  llvm::Type *ResType =
-  llvm::VectorType::get(llvm::Type::getInt32Ty(getVMContext()), 2);
+  auto *ResType =
+  llvm::FixedVectorType::get(llvm::Type::getInt32Ty(getVMContext()), 2);
   return ABIArgInfo::getDirect(ResType);
 }
 if (Size == 128) {
-  llvm::Type *ResType =
-  llvm::VectorType::get(llvm::Type::getInt32Ty(getVMContext()), 4);
+  auto *ResType =
+  llvm::FixedVectorType::get(llvm::Type::getInt32Ty(getVMContext()), 4);
   return ABIArgInfo::getDirect(ResType);
 }
 return getNaturalAlignIndirect(Ty, /*ByVal=*/false);
@@ -6209,7 +6210,7 @@
 return ABIArgInfo::getDirect(ResType);
   }
   if (Size == 64 || Size == 128) {
-llvm::Type *ResType = llvm::VectorType::get(
+auto *ResType = llvm::FixedVectorType::get(
 llvm::Type::getInt32Ty(getVMContext()), Size / 32);
 return ABIArgInfo::getDirect(ResType);
   }
@@ -6225,7 +6226,7 @@
 // FP16 vectors should be converted to integer vectors
 if (!getTarget().hasLegalHalfType() && containsAnyFP16Vectors(Ty)) {
   uint64_t Size = getContext().getTypeSize(VT);
-  llvm::Type *NewVecTy = llvm::VectorType::get(
+  auto *NewVecTy

[PATCH] D80323: [SVE] Eliminate calls to default-false VectorType::get() from Clang

2020-05-21 Thread Christopher Tetreault via Phabricator via cfe-commits 
ctetreau added a comment.

In D80323#2048457 , @rjmccall wrote:

> I'm sympathetic to wanting to get rid of the boolean flag, but this is a 
> really invasive change for pretty minimal benefit.  Why not leave 
> `VectorType::get` as meaning a non-scalable vector type and come up with a 
> different method name to get a scalable vector?


As it stands today, if you call VectorType::get(Ty, N, false), you will get an 
instance of FixedVectorType, via a base VectorType pointer. Currently, you can 
call getNumElements() on a base VectorType and it will work, but the ultimate 
endgame is that getNumElements is going to be removed from base VectorType. 
This change makes it so that you don't have to cast your VectorType object to 
FixedVectorType everywhere.

The overall architecture is that there is a derived type for each sort of 
vector. VectorType is the base class, and the two derived vector types are 
FixedVectorType and ScalableVectorType. I suppose I could have named them 
something like BaseVectorType, VectorType(actually fixed width vector type), 
and ScalableVectorType, but I feel that this is a worse solution because it's 
not obvious by the name what a "VectorType" is. Additionally, naming the types 
this would have broken all code that previously worked for scalable vectors. 
It's not obvious to me which naming scheme would have resulted in less changes 
(changes to rename fixed vectors to FixedVectorType vs changes needed to rename 
all generic code to BaseVectorType and to fix code for scalable vectors), but I 
think the consistency in the naming scheme justifies the path I chose.

For your specific proposal, I think it would be very strange if a function with 
the signature `static FixedVectorType *get(Type *, unsigned)` were a member of 
the base VectorType.

If you'd like to know more about the motivation for this work, here is a link 
to the RFC: http://lists.llvm.org/pipermail/llvm-dev/2020-March/139811.html


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80323



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

[PATCH] D80323: [SVE] Eliminate calls to default-false VectorType::get() from Clang

2020-05-21 Thread Francesco Petrogalli via Phabricator via cfe-commits 
fpetrogalli added a comment.

In D80323#2048457 , @rjmccall wrote:

> I'm sympathetic to wanting to get rid of the boolean flag, but this is a 
> really invasive change for pretty minimal benefit.  Why not leave 
> `VectorType::get` as meaning a non-scalable vector type and come up with a 
> different method name to get a scalable vector?


Sorry @rjmccall , I saw your comments only after approving this patch. I didn't 
mean to enforce my views over yours.

@ctetreau , please make sure you address @rjmccall comment before submitting.

Kind regards,

Francesco


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80323



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

[PATCH] D80323: [SVE] Eliminate calls to default-false VectorType::get() from Clang

2020-05-21 Thread Francesco Petrogalli via Phabricator via cfe-commits 
fpetrogalli accepted this revision.
fpetrogalli added a comment.
This revision is now accepted and ready to land.

LGTM, it would be great if you could address the two comments I added, before 
submitting.




Comment at: clang/lib/CodeGen/CGBuiltin.cpp:10775
 
-  llvm::VectorType *MaskTy = llvm::VectorType::get(CGF.Builder.getInt1Ty(),
- cast(Mask->getType())->getBitWidth());
+  llvm::VectorType *MaskTy = llvm::FixedVectorType::get(
+  CGF.Builder.getInt1Ty(),

`auto`?



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:4670
 if (!CGF.CGM.getCodeGenOpts().PreserveVec3Type) {
-  auto Vec4Ty = llvm::VectorType::get(
+  auto Vec4Ty = llvm::FixedVectorType::get(
   cast(DstTy)->getElementType(), 4);

`auto *`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80323



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

[PATCH] D80323: [SVE] Eliminate calls to default-false VectorType::get() from Clang

2020-05-21 Thread John McCall via Phabricator via cfe-commits 
rjmccall added a comment.

I'm sympathetic to wanting to get rid of the boolean flag, but this is a really 
invasive change for pretty minimal benefit.  Why not leave `VectorType::get` as 
meaning a non-scalable vector type and come up with a different method name to 
get a scalable vector?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80323



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

[PATCH] D80323: [SVE] Eliminate calls to default-false VectorType::get() from Clang

2020-05-20 Thread Christopher Tetreault via Phabricator via cfe-commits 
ctetreau created this revision.
Herald added subscribers: cfe-commits, dmgreen, psnobl, rkruppe, tschuett.
Herald added a reviewer: efriedma.
Herald added a project: clang.
ctetreau added reviewers: david-arm, fpetrogalli, ddunbar, rjmccall.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80323

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/SwiftCallingConv.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/utils/TableGen/MveEmitter.cpp

Index: clang/utils/TableGen/MveEmitter.cpp
===
--- clang/utils/TableGen/MveEmitter.cpp
+++ clang/utils/TableGen/MveEmitter.cpp
@@ -300,7 +300,7 @@
 return Element->cNameBase() + "x" + utostr(Lanes);
   }
   std::string llvmName() const override {
-return "llvm::VectorType::get(" + Element->llvmName() + ", " +
+return "llvm::FixedVectorType::get(" + Element->llvmName() + ", " +
utostr(Lanes) + ")";
   }
 
@@ -354,7 +354,7 @@
 // explanation.
 unsigned ModifiedLanes = (Lanes == 2 ? 4 : Lanes);
 
-return "llvm::VectorType::get(Builder.getInt1Ty(), " +
+return "llvm::FixedVectorType::get(Builder.getInt1Ty(), " +
utostr(ModifiedLanes) + ")";
   }
 
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -1478,8 +1478,8 @@
   // registers and we need to make sure to pick a type the LLVM
   // backend will like.
   if (Size == 128)
-return ABIArgInfo::getDirect(llvm::VectorType::get(
-  llvm::Type::getInt64Ty(getVMContext()), 2));
+return ABIArgInfo::getDirect(llvm::FixedVectorType::get(
+llvm::Type::getInt64Ty(getVMContext()), 2));
 
   // Always return in register if it fits in a general purpose
   // register, or if it is 64 bits and has a single element.
@@ -3122,8 +3122,8 @@
 cast(IRType)->getElementType()->isIntegerTy(128)) {
   // Use a vXi64 vector.
   uint64_t Size = getContext().getTypeSize(Ty);
-  return llvm::VectorType::get(llvm::Type::getInt64Ty(getVMContext()),
-   Size / 64);
+  return llvm::FixedVectorType::get(llvm::Type::getInt64Ty(getVMContext()),
+Size / 64);
 }
 
 return IRType;
@@ -3138,8 +3138,8 @@
 
 
   // Return a LLVM IR vector type based on the size of 'Ty'.
-  return llvm::VectorType::get(llvm::Type::getDoubleTy(getVMContext()),
-   Size / 64);
+  return llvm::FixedVectorType::get(llvm::Type::getDoubleTy(getVMContext()),
+Size / 64);
 }
 
 /// BitsContainNoUserData - Return true if the specified [start,end) bit range
@@ -3273,7 +3273,8 @@
   // case.
   if (ContainsFloatAtOffset(IRType, IROffset, getDataLayout()) &&
   ContainsFloatAtOffset(IRType, IROffset+4, getDataLayout()))
-return llvm::VectorType::get(llvm::Type::getFloatTy(getVMContext()), 2);
+return llvm::FixedVectorType::get(llvm::Type::getFloatTy(getVMContext()),
+  2);
 
   return llvm::Type::getDoubleTy(getVMContext());
 }
@@ -4140,8 +4141,8 @@
 
   // Mingw64 GCC returns i128 in XMM0. Coerce to v2i64 to handle that.
   // Clang matches them for compatibility.
-  return ABIArgInfo::getDirect(
-  llvm::VectorType::get(llvm::Type::getInt64Ty(getVMContext()), 2));
+  return ABIArgInfo::getDirect(llvm::FixedVectorType::get(
+  llvm::Type::getInt64Ty(getVMContext()), 2));
 
 default:
   break;
@@ -5478,13 +5479,13 @@
   return ABIArgInfo::getDirect(ResType);
 }
 if (Size == 64) {
-  llvm::Type *ResType =
-  llvm::VectorType::get(llvm::Type::getInt32Ty(getVMContext()), 2);
+  auto *ResType =
+  llvm::FixedVectorType::get(llvm::Type::getInt32Ty(getVMContext()), 2);
   return ABIArgInfo::getDirect(ResType);
 }
 if (Size == 128) {
-  llvm::Type *ResType =
-  llvm::VectorType::get(llvm::Type::getInt32Ty(getVMContext()), 4);
+  auto *ResType =
+  llvm::FixedVectorType::get(llvm::Type::getInt32Ty(getVMContext()), 4);
   return ABIArgInfo::getDirect(ResType);
 }
 return getNaturalAlignIndirect(Ty, /*ByVal=*/false);
@@ -6209,7 +6210,7 @@
 return ABIArgInfo::getDirect(ResType);
   }
   if (Size == 64 || Size == 128) {
-llvm::Type *ResType = llvm::VectorType::get(
+auto *ResType = llvm::FixedVectorType::get(
 llvm::Type::getInt32Ty(getVMContext()), Size / 32);
 return ABIArgInfo::getDirect(ResType);
   }
@@ -6225,7 +6226,7 @@
 // FP16 vectors should be converted to integer vectors
 if (!getTarget().hasLegalHalfType() && containsAnyFP16Vectors(Ty)) {
   uint64_t Size = getContext().getTypeSize(VT);
-