[PATCH] D72426: [GlobalISel][AArch64] Import + select LDR*roW and STR*roW patterns

[Amara Emerson via Phabricator via llvm-commits]

aemerson accepted this revision.
aemerson added a comment.
This revision is now accepted and ready to land.

LGTM with some minor comments.



================
Comment at: llvm/lib/Target/AArch64/AArch64InstructionSelector.cpp:4380
+  // Check if we can find the G_PTR_ADD.
+  MachineInstr *Gep = getOpcodeDef(TargetOpcode::G_PTR_ADD, Root.getReg(), MRI);
+  if (!Gep || !isWorthFoldingIntoExtendedReg(*Gep, MRI))
----------------
While we're here maybe rename this variable from Gep to PtrAdd?


================
Comment at: llvm/lib/Target/AArch64/AArch64InstructionSelector.cpp:4389
+  return selectExtendedSHL(Root, Gep->getOperand(1), OffsetInst->getOperand(0),
+                           SizeInBytes, false);
+}
----------------
Can you add a comment to the WantsExt parameter like: `/* WantsExt */`


================
Comment at: llvm/lib/Target/AArch64/AArch64InstructionSelector.cpp:4489
+  auto ExtendedShl = selectExtendedSHL(Root, LHS, OffsetInst->getOperand(0),
+                                       SizeInBytes, true);
+  if (ExtendedShl)
----------------
Same for WantsExt parameter here.


================
Comment at: llvm/test/CodeGen/AArch64/GlobalISel/load-wro-addressing-modes.mir:194
+    %foo:gpr(s32) = COPY $w1
+    %ext:gpr(s64) = G_ZEXT %foo(s32)
+    %c:gpr(s64) = G_CONSTANT i64 4
----------------
This test looks the same as the one above? Should the G_ZEXT be G_ANYEXT?


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

https://reviews.llvm.org/D72426