[PATCH] D119728: [AArch64][SVE] Handle more cases in findMoreOptimalIndexType.

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:16454
+    if (auto Offset = DAG.getSplatValue(Index.getOperand(1))) {
+      Offset = DAG.getNode(ISD::MUL, DL, MVT::i64, Offset, Scale);
+      BasePtr = DAG.getNode(ISD::ADD, DL, MVT::i64, BasePtr, Offset);
----------------
Is Index guaranteed to be a vector of i64s? For the general case I believe it can be any type so you'll either need an early exit if the scenario is unlikely or some kind of extension if it's something you care about, otherwise code like this will assert.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:16474-16475
+      if (auto Offset = DAG.getSplatValue(OffsetOp)) {
+        Offset = DAG.getNode(ISD::MUL, DL, MVT::i64, Offset, Scale);
+        Offset = DAG.getNode(ISD::SHL, DL, MVT::i64, Offset, Shift);
+        BasePtr = DAG.getNode(ISD::ADD, DL, MVT::i64, BasePtr, Offset);
----------------
I guess it doesn't matter arithmetically? but is there a reason to switch from the original `(Offset << Shift) * Scale` to `(Offset * Scale) << Shift`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119728