-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[VPlan] Fold NOT into predicate of wide compares. #129430
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
16041bb
89c97c7
f1fe3a1
2702792
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -972,8 +972,25 @@ static void simplifyRecipe(VPRecipeBase &R, VPTypeAnalysis &TypeInfo) { | |
if (match(&R, m_c_Mul(m_VPValue(A), m_SpecificInt(1)))) | ||
return R.getVPSingleValue()->replaceAllUsesWith(A); | ||
|
||
if (match(&R, m_Not(m_Not(m_VPValue(A))))) | ||
return R.getVPSingleValue()->replaceAllUsesWith(A); | ||
if (match(&R, m_Not(m_VPValue(A)))) { | ||
if (match(A, m_Not(m_VPValue(A)))) | ||
return R.getVPSingleValue()->replaceAllUsesWith(A); | ||
|
||
// Try to fold Not into compares by adjusting the predicate in-place. | ||
if (isa<VPWidenRecipe>(A) && A->getNumUsers() == 1) { | ||
auto *WideCmp = cast<VPWidenRecipe>(A); | ||
if (WideCmp->getOpcode() == Instruction::ICmp || | ||
WideCmp->getOpcode() == Instruction::FCmp) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've forgotten the rules somewhat regarding what's permitted with strict FP, but are there any issues with not(fcmp pred1) being transformed to fcmp pred2? For example, if an input is NaN? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. AFAIK the predicates should be symmetric, instcombine also folds the negation into the compare by adjusting the predicate for all cases: https://llvm.godbolt.org/z/a9KqeocWd I also checked most of the changes in the tests with Alive2: https://alive2.llvm.org/ce/z/WGDz9U There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for taking a look! |
||
WideCmp->setPredicate( | ||
CmpInst::getInversePredicate(WideCmp->getPredicate())); | ||
R.getVPSingleValue()->replaceAllUsesWith(WideCmp); | ||
// If WideCmp doesn't have a debug location, use the one from the | ||
// negation, to preserve the location. | ||
if (!WideCmp->getDebugLoc() && R.getDebugLoc()) | ||
WideCmp->setDebugLoc(R.getDebugLoc()); | ||
} | ||
} | ||
} | ||
|
||
// Remove redundant DerviedIVs, that is 0 + A * 1 -> A and 0 + 0 * x -> 0. | ||
if ((match(&R, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recipe expected to have a predicate, which this method replaces?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I presume this was meant to mirror the LLVM IR method in
CmpInst::setPredicate
. Is it a thing where we try to keep the methods onVPRecipe
in sync with theirInstruction
counterparts?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep this was to mirror the corresponding API at CmpInst.