[PATCH] D32995: [GlobalISel][X86] G_ICMP support.

[Guy Blank via Phabricator via llvm-commits]

guyblank added inline comments.


================
Comment at: lib/Target/X86/X86FastISel.cpp:183
 
-static std::pair<X86::CondCode, bool>
-getX86ConditionCode(CmpInst::Predicate Predicate) {
----------------
perhaps commit this separately as an NFC?


================
Comment at: lib/Target/X86/X86InstrInfo.h:67
 
+/// \brief Return a pair of condition code for the given predicate and wheter
+/// the instruction operands should be swaped to match the condition code.
----------------
wheter -> whether


================
Comment at: lib/Target/X86/X86InstructionSelector.cpp:627
+  bool SwapArgs;
+  std::tie(CC, SwapArgs) = X86::getX86ConditionCode((CmpInst::Predicate)I.getOperand(1).getPredicate());
+  assert(CC <= X86::LAST_VALID_COND && "Unexpected condition code.");
----------------
indentation


================
Comment at: lib/Target/X86/X86InstructionSelector.cpp:628
+  std::tie(CC, SwapArgs) = X86::getX86ConditionCode((CmpInst::Predicate)I.getOperand(1).getPredicate());
+  assert(CC <= X86::LAST_VALID_COND && "Unexpected condition code.");
+  unsigned OpSet = X86::getSETFromCond(CC);
----------------
getSETFromCond asserts this already


================
Comment at: lib/Target/X86/X86LegalizerInfo.cpp:93
     setAction({G_SEXT, 1, Ty}, Legal);
   }
+
----------------
unrelated to this patch, but can we have the common legalization info for 32/64 bit in one function?


https://reviews.llvm.org/D32995