[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