[PATCH] D89422: [AArch64][GlobalISel] Swap compare operands when it may be profitable

[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.



================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp:462
+  bool IsLegal = (C >> 12 == 0) || ((C & 0xFFFULL) == 0 && C >> 24 == 0);
+  LLVM_DEBUG(dbgs() << "Is imm " << C
+                    << " legal: " << (IsLegal ? "yes\n" : "no\n"));
----------------
This will probably print too often in output.


================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp:3940
+  if (IsSupportedExtend(*ShiftLHS))
+    return (Shift <= 4) ? 2 : 1;
+  LLT Ty = MRI.getType(Def->getOperand(0).getReg());
----------------
Comment to explain why '4' and '2' and '1'?


================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp:3941-3943
+  LLT Ty = MRI.getType(Def->getOperand(0).getReg());
+  if (Ty.isVector())
+    return 0;
----------------
I think this should be an earlier exit check.


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

https://reviews.llvm.org/D89422