[PATCH] D65024: [GlobalISel][AArch64] Contract trivial same-size cross-bank copies into G_STOREs

[Jessica Paquette via Phabricator via llvm-commits]

paquette marked 3 inline comments as done.
paquette added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstructionSelector.cpp:1170
+  assert(I.getOpcode() == TargetOpcode::G_STORE && "Expected G_STORE");
+  // If we're storing 32 bits, it doesn't matter if it's 32 bits on a FPR or a
+  // GPR. So, if we run into something like this:
----------------
aemerson wrote:
> You mention 32 bits here but the example uses 64 bit types. Also, it doesn't matter whether we use 32 or 64 bit stores right?
Right, it doesn't matter. I'll update the comment for consistency though.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstructionSelector.cpp:1184
+  if (!Copy || Copy->getOpcode() != TargetOpcode::COPY)
+    return;
+  Register CopyDstReg = Copy->getOperand(0).getReg();
----------------
aemerson wrote:
> This pattern can now be replaced with getOpcodeDef()?
It's not quite equivalent, since it looks through copies. I think that it would still work though, if I rejigger the rest of the function a bit.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstructionSelector.cpp:1276
+  case TargetOpcode::G_STORE:
+    contractCrossBankCopyIntoStore(I, MRI);
+    return false;
----------------
aemerson wrote:
> You're returning false here because we want selection to continue, even though you changed the instruction. This is what the preISelLower() hook I added is for.
👍


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

https://reviews.llvm.org/D65024