[PATCH] D86709: [GlobalISel] Extend not_cmp_fold to work on conditional expressions

[Jessica Paquette via Phabricator via llvm-commits]

paquette added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:2154
 
-  // Now try match src to either icmp or fcmp.
-  if (!mi_match(XorSrc, MRI, m_GICmp(m_Pred(), m_Reg(), m_Reg()))) {
-    // Try fcmp.
-    if (!mi_match(XorSrc, MRI, m_GFCmp(m_Pred(), m_Reg(), m_Reg())))
+  // Check that XorSrc is the root of a tree of comparisons combined with ANDs
+  // and ORs. The suffix of RegsToNegate starting from index I is used a work
----------------
A MIR-esque example would be handy here, just to make it explicit what's being matched.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:2155
+  // Check that XorSrc is the root of a tree of comparisons combined with ANDs
+  // and ORs. The suffix of RegsToNegate starting from index I is used a work
+  // list of tree nodes to visit.
----------------
I don't think here's any reason to talk about an index `I` here; from the looks of it, the below loop can be a range-based for?


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:2162
       return false;
+    MachineInstr *Def = MRI.getVRegDef(Reg);
+    switch (Def->getOpcode()) {
----------------
Usually people assert that the result of `MRI.getVRegDef(...)` isn't null.

I'm not sure if that's actually necessary in this case though; I *think* that it can only be null when you have a physreg, but I'm not entirely sure.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:2193
+    // For each comparison, invert the opcode. For each AND and OR, change the
+    // opcode.
+    switch (Def->getOpcode()) {
----------------
A MIR-esque example would be useful here too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86709