[PATCH] D64084: [AArch64][GlobalISel] Teach tryOptSelect to handle G_ICMP

[Amara Emerson via Phabricator via llvm-commits]

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

Nice cleanup, some minor comments.



================
Comment at: llvm/lib/Target/AArch64/AArch64InstructionSelector.cpp:2777
+                                    MachineIRBuilder &MIRBuilder) const {
+  assert(LHS.isReg() && RHS.isReg() && "Expected LHS and RHS to be registers!");
+  MachineRegisterInfo &MRI = MIRBuilder.getMF().getRegInfo();
----------------
Can we just make these emit interfaces use Register instead? It doesn't seem like we need the full generality of MachineOperand.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstructionSelector.cpp:3052
+AArch64InstructionSelector::tryOptCMN(MachineOperand &LHS, MachineOperand &RHS,
+                                      MachineOperand &Predicate,
+                                      MachineIRBuilder &MIRBuilder) const {
----------------
And the same with Predicate too, we can just pass the actual CmpInst::Predicate value and avoid further casts,


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

https://reviews.llvm.org/D64084