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

Sanjoy Das sanjoy at playingwithpointers.com
Wed Apr 29 21:21:18 PDT 2015


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

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

================
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;
+    };
+  }();
+
----------------
majnemer wrote:
> 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?
> How about we just split it out to another function?

done

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

================
Comment at: lib/Transforms/InstCombine/InstCombineSelect.cpp:903
@@ +902,3 @@
+
+  // SPF_2(SPF_1(~A, ~B), ~C) == ~!SPF_2(!SPF_1(A, B), C)
+  //
----------------
majnemer wrote:
> 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.
The abstract pattern does not abstract over very much. :)  I'll just document the four possibilities explicitly.

http://reviews.llvm.org/D9353

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






More information about the llvm-commits mailing list