sdesmalen added a comment.

In D131547#3723083 <https://reviews.llvm.org/D131547#3723083>, @dmgreen wrote:

> I think we usually try to do the same, if the intrinsics have been in 
> released compilers. There is an example in 
> https://reviews.llvm.org/D98487#change-tOTTgECYYAO5, hopefully these would be 
> equally simple.

We don't really have the intention in keeping compatibility for intrinsics like 
these, since the vector.extract/insert intrinsics are the proper way to 
insert/extract vectors and they have been around for quite some time now. Also 
these intrinsics are quite specific to the initial SVE ACLE implementation when 
we first upstreamed it, so it's not very likely that anyone else is using them.



================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:9085-9097
+  auto *VTy = dyn_cast<llvm::VectorType>(Ty);
+  unsigned I = cast<ConstantInt>(Ops[1])->getSExtValue();
+
+ if (TypeFlags.isTupleSet()){
+   auto *SrcTy = dyn_cast<llvm::VectorType>(Ops[2]->getType());
+   unsigned MinElts = SrcTy->getElementCount().getKnownMinValue();
+   Value *Idx = ConstantInt::get(CGM.Int64Ty, I * MinElts);
----------------
I think you could simplify this further to:

   auto *SingleVecTy = cast<llvm::ScalableVectorType>(
       TypeFlags.isTupleSet() ? Ops[2]->getType() : Ty);

   unsigned I = cast<ConstantInt>(Ops[1])->getSExtValue();
   Value *Idx =
       ConstantInt::get(CGM.Int64Ty, I * SingleVecTy->getMinNumElements());
  
   if (TypeFlags.isTupleSet())
     return Builder.CreateExtractVector(Ty, Ops[0], Ops[2], Idx);
   else
     return Builder.CreateInsertVector(Ty, Ops[0], Idx);



================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:9106
+
+  auto *VTy = dyn_cast<llvm::VectorType>(Ty);
+  auto *SrcTy = dyn_cast<llvm::VectorType>(Ops[0]->getType());
----------------
Casting to `VectorType` here isn't actually necessary, because both 
`llvm::PoisonValue::get()` and `Builder.CreateInsertVector()` take a `Type*`.


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:9107
+  auto *VTy = dyn_cast<llvm::VectorType>(Ty);
+  auto *SrcTy = dyn_cast<llvm::VectorType>(Ops[0]->getType());
+  unsigned MinElts = SrcTy->getElementCount().getKnownMinValue();
----------------
nit: If you cast this to `llvm::ScalableVectorType` you can call 
`SrcTy->getMinNumElements()` directly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131547

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

Reply via email to