[PATCH] D127903: [InstCombine] Optimize test for same-sign of values
Sanjay Patel via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sun Jun 19 07:21:10 PDT 2022
spatel added a comment.
I committed the tests with baseline results, so we have those in place independently of the code change. I then updated the tests to provide a bit more coverage. Please rebase/update after:
feb4b336acc71 <https://reviews.llvm.org/rGfeb4b336acc7100fbb87fdd8f61f1d58cd234709>
It's programmer preference, but I think it'd be easier to read with less IsAnd ? () :() constructs, so something like this:
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
index cc06e2fd45db..4f8814475e12 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
@@ -2647,6 +2647,38 @@ Value *InstCombinerImpl::foldAndOrOfICmps(ICmpInst *LHS, ICmpInst *RHS,
}
}
+ // Match naive pattern (and its inverted form) for checking if two values
+ // share same sign. An example of the pattern: (icmp slt (X & Y), 0) | (icmp
+ // sgt (X | Y), -1) -> (icmp sgt (X ^ Y), -1) Inverted form (example): (icmp
+ // slt (X | Y), 0) & (icmp sgt (X & Y), -1) -> (icmp slt (X ^ Y), 0)
+ bool TrueIfSignedL, TrueIfSignedR;
+ if (InstCombiner::isSignBitCheck(PredL, *LHSC, TrueIfSignedL) &&
+ InstCombiner::isSignBitCheck(PredR, *RHSC, TrueIfSignedR) &&
+ (RHS->hasOneUse() || LHS->hasOneUse())) {
+ Value *X, *Y;
+ if (IsAnd) {
+ if ((TrueIfSignedL && !TrueIfSignedR &&
+ match(LHS0, m_Or(m_Value(X), m_Value(Y))) &&
+ match(RHS0, m_c_And(m_Specific(X), m_Specific(Y)))) ||
+ (!TrueIfSignedL && TrueIfSignedR &&
+ match(LHS0, m_And(m_Value(X), m_Value(Y))) &&
+ match(RHS0, m_c_Or(m_Specific(X), m_Specific(Y))))) {
+ Value *NewXor = Builder.CreateXor(X, Y);
+ return Builder.CreateIsNeg(NewXor);
+ }
+ } else {
+ if (((TrueIfSignedL && !TrueIfSignedR &&
+ match(LHS0, m_And(m_Value(X), m_Value(Y))) &&
+ match(RHS0, m_c_Or(m_Specific(X), m_Specific(Y))))) ||
+ (!TrueIfSignedL && TrueIfSignedR &&
+ match(LHS0, m_Or(m_Value(X), m_Value(Y))) &&
+ match(RHS0, m_c_And(m_Specific(X), m_Specific(Y))))) {
+ Value *NewXor = Builder.CreateXor(X, Y);
+ return Builder.CreateIsNotNeg(NewXor);
+ }
+ }
+ }
+
return foldAndOrOfICmpsUsingRanges(LHS, RHS, IsAnd);
}
================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2640
+ InstCombiner::isSignBitCheck(PredR, *RHSC, TrueIfSignedR) &&
+ RHS->hasOneUse() && LHS->hasOneUse()) {
+ Value *X, *Y;
----------------
The use checks are too strict. This optimization should be a win as long as any one of the intermediate values has one use. We are creating 2 new values, so as long as we can remove one of the old ones + the final value, it is an improvement.
I'd make this:
(LHS->hasOneUse() || RHS->hasOneUse())
..and remove the use checks for LHS0 and RHS0 under here.
================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2654-2658
+ Value *NewC = IsAnd ? ConstantInt::getNullValue(NewXor->getType())
+ : ConstantInt::getAllOnesValue(NewXor->getType());
+ ICmpInst::Predicate NewPred =
+ IsAnd ? ICmpInst::ICMP_SLT : ICmpInst::ICMP_SGT;
+ return Builder.CreateICmp(NewPred, NewXor, NewC);
----------------
There's a Builder shortcut to create the new ICmp: `CreateIsNeg` / `CreateIsNotNeg`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127903/new/
https://reviews.llvm.org/D127903
More information about the llvm-commits
mailing list