[PATCH] D91755: [GlobalISel] Combine icmp eq/ne (ext (icmp cc, x, y)), true/false)

[Amara Emerson via Phabricator via llvm-commits]

aemerson added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:3149-3150
+  // Only do this pre-legalization.
+  if (LI)
+    return false;
+  LLT s1 = LLT::scalar(1);
----------------
I'm not convinced this is necessary. If legality is something that a target should care about then I think they shouldn't add this combine post-legalization.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:3172
+  // We're going to look for a second compare which feeds into the extend.
+  Register TopCmp;
+  bool IsSignExtended = mi_match(ExtReg, MRI, m_GSExt(m_Reg(TopCmp)));
----------------
SrcCmp is a better name?


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:3188
+  // Match: icmp cc, x, y
+  Register TopCmpLHS, TopCmpRHS;
+  CmpInst::Predicate TopCmpPred;
----------------
Same here.


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

https://reviews.llvm.org/D91755