[PATCH] D150360: [InstCombine] Optimize compares with multiple selects as operands
Nikita Popov via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu May 25 00:58:04 PDT 2023
nikic added inline comments.
================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:6597
+ match(Op1, m_Select(m_Specific(Cond), m_Value(C), m_Value(D))) &&
+ Op0->hasOneUse() && Op1->hasOneUse()) {
+ // Check whether comparison of TrueValues can be simplified
----------------
There are two ways the one-use checks could be relaxed:
1. It should be fine to only require one-use on one of the selects, as we'll preserve the number of instructions in that case. I'm not entirely clear on profitability for that case -- I guess the fact that we're converting an arbitrary select into a logical and/or may make it generally beneficial?
2. If both comparisons fold, we don't care about uses at all, as the result is a constant.
I personally think it would be okay to start with the code you have, but in that case we should at least add a TODO to note the possibility.
================
Comment at: llvm/test/Transforms/InstCombine/icmp-with-selects.ll:4
+
+define i1 @foo(i32 %param, i1 %cond) {
+; CHECK-LABEL: define i1 @foo
----------------
Could you please give these tests some more meaningful names, for example this could be something like `@both_sides_fold`.
================
Comment at: llvm/test/Transforms/InstCombine/icmp-with-selects.ll:126
+ %cond6 = select i1 %cond, i32 %val2, i32 %param
+ %shl = shl i32 %cond1, 4
+ %cmp = icmp sgt i32 %cond6, %cond1
----------------
For multi-use, it's customary to just do a `call void @use(i32 %cond1)`. This is more obvious than trying to come up with a way to combine the uses.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D150360/new/
https://reviews.llvm.org/D150360
More information about the llvm-commits
mailing list