[PATCH] D157628: [AArch64][SVE2] Change the cost of extends with S/URHADD to 0

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:2053
+//
+bool isExtShiftRightAdd(const Instruction *ExtUser, const CastInst *Ext,
+                        Type *Dst, Type *Src) {
----------------
nit: is it worth renaming this to `isExtPartOfAvgExpr` ?


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:2062
+  const Instruction *Add = ExtUser;
+  auto *AddUser = dyn_cast<Instruction>(Add->getUniqueUndroppableUser());
+  if (AddUser && AddUser->getOpcode() == Instruction::Add)
----------------
`getUniqueUndroppableUser()` can return `nullptr` if there isn't a unique and droppable user, so this should use `dyn_cast_or_null`.
I guess this is also missing a negative test-case?


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:2084
+  // Ensure both extends are of the same type
+  Instruction *Ex1User = cast<CastInst>(Ex1->getUniqueUndroppableUser());
+  Instruction *Ex2User = cast<CastInst>(Ex2->getUniqueUndroppableUser());
----------------
Rather than checking for `getUniqueUndroppableUser()` again here, maybe you can generalise the match expression using `m_Instruction(Ext1)` and then add a new match expression to see if `m_ZExtOrSext(Ext1)` and `Ext1->getOpcode() == Ext2->getOpcode()`


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

https://reviews.llvm.org/D157628