[PATCH] D69716: NeonEmitter: remove special 'a' modifier.

[Tim Northover via Phabricator via cfe-commits]

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

Thanks:

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

  6c2a4f5ff93..59f063b89c5  master -> master


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69716



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

[PATCH] D69716: NeonEmitter: remove special 'a' modifier.

[Eli Friedman via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69716



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

[PATCH] D69716: NeonEmitter: remove special 'a' modifier.

[Tim Northover via Phabricator via cfe-commits]

t.p.northover created this revision.
t.p.northover added a reviewer: efriedma.
Herald added a subscriber: mcrosier.
Herald added a project: clang.

This has been separated off from D69618  to 
reduce diff clutter, as it really stands on its own. The 'a' modifier is rarely 
used, but emits splatting code from NeonEmitter.cpp when it is. This 
functionality can be implemented directly in .td files though, at a net 
reduction in lines of code.

There are a few test changes, but they're just moving bitcasts around due to 
the slightly different input Clang sees. Nothing really changes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69716

Files:
  clang/include/clang/Basic/arm_neon.td
  clang/include/clang/Basic/arm_neon_incl.td
  clang/test/CodeGen/aarch64-neon-2velem.c
  clang/test/CodeGen/arm_neon_intrinsics.c
  clang/utils/TableGen/NeonEmitter.cpp

Index: clang/utils/TableGen/NeonEmitter.cpp
===
--- clang/utils/TableGen/NeonEmitter.cpp
+++ clang/utils/TableGen/NeonEmitter.cpp
@@ -412,17 +412,6 @@
 return Idx - 1;
   }
 
-  /// Return true if the intrinsic takes an splat operand.
-  bool hasSplat() const { return Proto.find('a') != std::string::npos; }
-
-  /// Return the parameter index of the splat operand.
-  unsigned getSplatIdx() const {
-assert(hasSplat());
-unsigned Idx = Proto.find('a');
-assert(Idx > 0 && "Can't return a splat!");
-return Idx - 1;
-  }
-
   unsigned getNumParams() const { return Proto.size() - 1; }
   Type getReturnType() const { return Types[0]; }
   Type getParamType(unsigned I) const { return Types[I + 1]; }
@@ -431,7 +420,6 @@
   std::string getProto() const { return Proto; }
 
   /// Return true if the prototype has a scalar argument.
-  /// This does not return true for the "splat" code ('a').
   bool protoHasScalar() const;
 
   /// Return the index that parameter PIndex will sit at
@@ -950,7 +938,6 @@
 NumVectors = 0;
 break;
   case 's':
-  case 'a':
 Bitwidth = ElementBitwidth;
 NumVectors = 0;
 break;
@@ -1354,8 +1341,6 @@
   }
 }
 
-// We don't check 'a' in this function, because for builtin function the
-// argument matching to 'a' uses a vector type splatted from a scalar type.
 bool Intrinsic::protoHasScalar() const {
   return (Proto.find('s') != std::string::npos ||
   Proto.find('z') != std::string::npos ||
@@ -1374,12 +1359,6 @@
   bool SRet = getReturnType().getNumVectors() >= 2;
 
   StringRef N = Name;
-  if (hasSplat()) {
-// Call the non-splat builtin: chop off the "_n" suffix from the name.
-assert(N.endswith("_n"));
-N = N.drop_back(2);
-  }
-
   ClassKind LocalCK = CK;
   if (!protoHasScalar())
 LocalCK = ClassB;
@@ -1413,21 +1392,8 @@
   continue;
 }
 
-std::string Arg;
+std::string Arg = V.getName();
 Type CastToType = T;
-if (hasSplat() && I == getSplatIdx()) {
-  Arg = "(" + BaseType.str() + ") {";
-  for (unsigned J = 0; J < BaseType.getNumElements(); ++J) {
-if (J != 0)
-  Arg += ", ";
-Arg += V.getName();
-  }
-  Arg += "}";
-
-  CastToType = BaseType;
-} else {
-  Arg = V.getName();
-}
 
 // Check if an explicit cast is needed.
 if (CastToType.isVector()) {
@@ -2091,10 +2057,6 @@
   for (auto *Def : Defs) {
 if (Def->hasBody())
   continue;
-// Functions with 'a' (the splat code) in the type prototype should not get
-// their own builtin as they use the non-splat variant.
-if (Def->hasSplat())
-  continue;
 
 std::string S = "BUILTIN(__builtin_neon_" + Def->getMangledName() + ", \"";
 
@@ -2131,10 +2093,6 @@
 // __builtin_neon_* so we don't need to generate a definition for it.
 if (Def->hasBody())
   continue;
-// Functions with 'a' (the splat code) in the type prototype should not get
-// their own builtin as they use the non-splat variant.
-if (Def->hasSplat())
-  continue;
 // Functions which have a scalar argument cannot be overloaded, no need to
 // check them if we are emitting the type checking code.
 if (Def->protoHasScalar())
@@ -2215,10 +2173,6 @@
   for (auto *Def : Defs) {
 if (Def->hasBody())
   continue;
-// Functions with 'a' (the splat code) in the type prototype should not get
-// their own builtin as they use the non-splat variant.
-if (Def->hasSplat())
-  continue;
 // Functions which do not have an immediate do not need to have range
 // checking code emitted.
 if (!Def->hasImmediate())
Index: clang/test/CodeGen/arm_neon_intrinsics.c
===
--- clang/test/CodeGen/arm_neon_intrinsics.c
+++ clang/test/CodeGen/arm_neon_intrinsics.c
@@ -8472,11 +8472,11 @@
 }
 
 // CHECK-LABEL: @test_vmull_n_s16(
-// CHECK:   [[TMP0:%.*]] = bitcast <4 x i16> %a to <8 x i8>
 // CHECK:   [[VECINIT_I:%.*]] = insertelement