[PATCH] D67549: [IntrinsicEmitter] Add overloaded types for SVE intrinsics (Subdivide2 & Subdivide4)

[Diana Picus via Phabricator via cfe-commits]

rovka added inline comments.


================
Comment at: include/llvm/IR/DerivedTypes.h:515
+      else
+        VTy = VectorType::getTruncatedElementVectorType(VTy);
+    }
----------------
Why not move this logic to getTruncatedElementVectorType and drop getNarrowerFpElementVectorType altogether? 


================
Comment at: include/llvm/IR/Intrinsics.h:130
+             Kind == PtrToElt || Kind == VecElementArgument ||
+             Kind == Subdivide2Argument || Kind == Subdivide4Argument);
       return Argument_Info >> 3;
----------------
Not directly related to your change: Is there a reason why VecOfAnyPtrsToElt isn't in this assert? If not, maybe this is a good time to add a First/LastArgumentKind and just check that Kind is in the range.


================
Comment at: lib/IR/Function.cpp:989
+    Type *Ty = Tys[D.getArgumentNumber()];
+    if (VectorType *VTy = dyn_cast<VectorType>(Ty))
+      return VectorType::getSubdividedVectorType(VTy, 1);
----------------
Maybe replace this with an assert and drop the unreachable.


================
Comment at: lib/IR/Function.cpp:997
+      return VectorType::getSubdividedVectorType(VTy, 2);
+    llvm_unreachable("unhandled");
+  }
----------------
Ditto,


================
Comment at: lib/IR/Function.cpp:1312
+    case IITDescriptor::Subdivide4Argument: {
+      // This may only be used when referring to a previous vector argument.
+      if (D.getArgumentNumber() >= ArgTys.size())
----------------
Can you share this code? Either a small helper or a fallthrough with a repeat check on D.Kind to get the number of subdivisions would work.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67549