[PATCH] D94883: [CodeGen][SelectionDAG]Add new intrinsic experimental.vector.reverse

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/include/llvm/IR/Intrinsics.td:1627
+
+def int_experimental_vector_reverse : Intrinsic<[llvm_anyvector_ty],
+                                   [LLVMMatchType<0>],
----------------
Should this use DefaultAttrsIntrinsic?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:99
                          Res = PromoteIntRes_EXTRACT_SUBVECTOR(N); break;
+  case ISD::VECTOR_REVERSE:
+    Res = PromoteIntRes_VECTOR_REVERSE(N);
----------------
Can we keep consistent formatting with the rest of this switch?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:10827
+  SmallVector<int, 8> Mask;
+  for (unsigned i = 0; i != VT.getVectorNumElements(); ++i)
+    Mask.push_back(VT.getVectorNumElements() - 1 - i);
----------------
Put VT.getVectorNumElements() in a variable since its used here and in the loop body.


================
Comment at: llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td:253
+
 def SDT_AArch64Rev   : SDTypeProfile<1, 1, [SDTCisVec<0>, SDTCisSameAs<0,1>]>;
+def AArch64rev       : SDNode<"ISD::VECTOR_REVERSE", SDT_AArch64Rev>;
----------------
Why not put this in TargetSelectionDAG.td?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94883