[PATCH] [InstCombine] Add new rule for MIN(MAX(~A, ~B), ~C) et. al.

David Majnemer david.majnemer at gmail.com
Wed Apr 29 20:55:08 PDT 2015


================
Comment at: lib/Transforms/InstCombine/InstCombineSelect.cpp:24
@@ +23,3 @@
+static SelectPatternFlavor
+GetInverseMinMaxSelectPattern(SelectPatternFlavor SPF) {
+  switch (SPF) {
----------------
`getInverseMinMaxSelectPattern` would be more conforming to the coding standards.

================
Comment at: lib/Transforms/InstCombine/InstCombineSelect.cpp:40
@@ +39,3 @@
+
+static Value *GenerateMinMaxSelectPattern(InstCombiner::BuilderTy *Builder,
+                                          SelectPatternFlavor SPF, Value *A,
----------------
Let's call this `generateMinMaxSelectPattern` instead.

================
Comment at: lib/Transforms/InstCombine/InstCombineSelect.cpp:43-57
@@ +42,17 @@
+                                          Value *B) {
+  CmpInst::Predicate Pred = [&]() {
+    switch (SPF) {
+    default:
+      llvm_unreachable("unhandled!");
+
+    case SPF_SMIN:
+      return ICmpInst::ICMP_SLT;
+    case SPF_UMIN:
+      return ICmpInst::ICMP_ULT;
+    case SPF_SMAX:
+      return ICmpInst::ICMP_SGT;
+    case SPF_UMAX:
+      return ICmpInst::ICMP_UGT;
+    };
+  }();
+
----------------
It might be late but why the lambda? I guess it makes the control flow a little nicer. How about we just split it out to another function?

================
Comment at: lib/Transforms/InstCombine/InstCombineSelect.cpp:902
@@ +901,3 @@
+
+
+  // SPF_2(SPF_1(~A, ~B), ~C) == ~!SPF_2(!SPF_1(A, B), C)
----------------
Can we kill this newline?

================
Comment at: lib/Transforms/InstCombine/InstCombineSelect.cpp:903
@@ +902,3 @@
+
+  // SPF_2(SPF_1(~A, ~B), ~C) == ~!SPF_2(!SPF_1(A, B), C)
+  //
----------------
The other transforms are a little more concrete in that they are described with a specific `SelectPatternFlavor` and an actual value for the constant.  I think the abstract pattern would still be good to keep around though.

Speaking of which, I'm going to have to admit that I am not quite sure what `~!` means exactly.

http://reviews.llvm.org/D9353

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list