[PATCH] D93411: [GlobalISel] Transform sext (cmp pred, x, y) -> select (cmp pred, x, y) tval, 0

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:3151
+  LLT ExtTy = MRI.getType(MI.getOperand(0).getReg());
+  if (ExtTy.isVector() ||
+      getTargetLowering().convertSelectOfConstantsToMath(ExtTy))
----------------
 Vectors should be easy to handle?


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:3152
+  if (ExtTy.isVector() ||
+      getTargetLowering().convertSelectOfConstantsToMath(ExtTy))
+    return false;
----------------
This doesn't check the inputs are constants? The name seems wrong


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:3163-3173
+  // Depending on the result type of the compare, we may need a different
+  // true value.
+  // E.g. let's say we have
+  //
+  // %cmp:_(s32) = G_ICMP pred, %x, %y
+  // %sext:_(s64) = G_SEXT %cmp
+  //
----------------
This comment is pretty wordy for what it's explaining. I would expect this to defer to somewhere else to explain the concept of the bool true value


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:3173
+  //
+  // If %cmp produced a s1, however, then %sext would be -1.
+  MatchInfo = (CmpTy == LLT::scalar(1))
----------------
s/a/an/


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

https://reviews.llvm.org/D93411