[PATCH] D66835: [AArch64][GlobalISel] Select arithmetic extended register patterns

[Amara Emerson via Phabricator via llvm-commits]

aemerson added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstructionSelector.cpp:4590
+/// Instructions that accept extend modifiers like UXTW expect the register
+/// being extended to be a GPR32, but the incoming DAG might be acting on a
+/// GPR64 (either via SEXT_INREG or AND). Extract the appropriate low bits if
----------------
DAG?


================
Comment at: llvm/lib/Target/AArch64/AArch64InstructionSelector.cpp:4601
+  Register NarrowReg = MRI.createVirtualRegister(&AArch64::GPR32RegClass);
+  auto Copy = MIB.buildCopy({NarrowReg}, {ExtReg});
+  return Copy.getReg(0);
----------------
This is just doing a normal copy, not a sub register copy. I.e. here ExtReg could be s64 and NarrowReg is a GPR32 RC, which isn't legal.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstructionSelector.cpp:4658
+      if (ExtInst && ExtInst->getOpcode() != TargetOpcode::COPY &&
+          ExtInst->getOpcode() != TargetOpcode::G_TRUNC)
+        return None;
----------------
We should clean this up to have a GISel equivalent of `isDef32()` and add G_PHI.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstructionSelector.cpp:4663
+
+  // We require a GPR32 here. Narrow the ExtReg if needed using EXTRACT_SUBREG.
+  MachineIRBuilder MIB(*RootDef);
----------------
s/EXTRACT_SUBREG/subregister copy


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

https://reviews.llvm.org/D66835