-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[VPlan] Treat VPVector(End)PointerRecipe as single-scalar, if ops are. #169249
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
Conversation
VPVector(End)PointerRecipes are single-scalar if all their operands are. This should be effectively NFC currently, but it should re-enable cost checking for some more VPWidenMemoryRecipe after llvm#157387 as discovered by @john-brawn-arm.
|
@llvm/pr-subscribers-llvm-transforms Author: Florian Hahn (fhahn) ChangesVPVector(End)PointerRecipes are single-scalar if all their operands are. This should be effectively NFC currently, but it should re-enable cost checking for some more VPWidenMemoryRecipe after Full diff: https://github.com/llvm/llvm-project/pull/169249.diff 1 Files Affected:
diff --git a/llvm/lib/Transforms/Vectorize/VPlanUtils.cpp b/llvm/lib/Transforms/Vectorize/VPlanUtils.cpp
index 939216fe162a4..334ad973c5428 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanUtils.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanUtils.cpp
@@ -185,7 +185,8 @@ bool vputils::isSingleScalar(const VPValue *VPV) {
all_of(Rep->operands(), isSingleScalar));
}
if (isa<VPWidenGEPRecipe, VPDerivedIVRecipe, VPBlendRecipe,
- VPWidenSelectRecipe>(VPV))
+ VPWidenSelectRecipe, VPVectorPointerRecipe, VPVectorEndPointerRecipe>(
+ VPV))
return all_of(VPV->getDefiningRecipe()->operands(), isSingleScalar);
if (auto *WidenR = dyn_cast<VPWidenRecipe>(VPV)) {
return preservesUniformity(WidenR->getOpcode()) &&
|
|
@llvm/pr-subscribers-vectorizers Author: Florian Hahn (fhahn) ChangesVPVector(End)PointerRecipes are single-scalar if all their operands are. This should be effectively NFC currently, but it should re-enable cost checking for some more VPWidenMemoryRecipe after Full diff: https://github.com/llvm/llvm-project/pull/169249.diff 1 Files Affected:
diff --git a/llvm/lib/Transforms/Vectorize/VPlanUtils.cpp b/llvm/lib/Transforms/Vectorize/VPlanUtils.cpp
index 939216fe162a4..334ad973c5428 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanUtils.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanUtils.cpp
@@ -185,7 +185,8 @@ bool vputils::isSingleScalar(const VPValue *VPV) {
all_of(Rep->operands(), isSingleScalar));
}
if (isa<VPWidenGEPRecipe, VPDerivedIVRecipe, VPBlendRecipe,
- VPWidenSelectRecipe>(VPV))
+ VPWidenSelectRecipe, VPVectorPointerRecipe, VPVectorEndPointerRecipe>(
+ VPV))
return all_of(VPV->getDefiningRecipe()->operands(), isSingleScalar);
if (auto *WidenR = dyn_cast<VPWidenRecipe>(VPV)) {
return preservesUniformity(WidenR->getOpcode()) &&
|
Mel-Chen
left a comment
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.
Could we directly return true, like VPReductionRecipe?
Wrong person mentioned? Should be @ElvisWang123? |
fhahn
left a comment
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.
after
Could we directly return true, like VPReductionRecipe?
Yep that works, updated, thanks
#157387 as discovered by @john-brawn-arm.
Wrong person mentioned? Should be @ElvisWang123?
The mention should be correct, @ElvisWang123 landed #157387, but @john-brawn-arm discovered the issue. I'll strip to the username mentions from the commit message, to avoid excessive Github notifications
|
Hi @fhahn, this seems to trigger assertion failures in Clang: https://gcc.godbolt.org/z/87jE85Tdq A reduced test case: |
llvm#169249) VPVector(End)PointerRecipes are single-scalar if all their operands are. This should be effectively NFC currently, but it should re-enable cost checking for some more VPWidenMemoryRecipe after llvm#157387 as discovered by John Brawn.
llvm#169249) VPVector(End)PointerRecipes are single-scalar if all their operands are. This should be effectively NFC currently, but it should re-enable cost checking for some more VPWidenMemoryRecipe after llvm#157387 as discovered by John Brawn.
|
@fhahn a few more changes have landed on top of this one and it's become quite difficult for us to revert locally. At this point reverting this upstream is similarly non-trivial. Do you have an ETA for a fix? |
fhahn
left a comment
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.
This should have been fixed as of last week by 1054a6e. The reproducer is not crashing any longer on current main: https://gcc.godbolt.org/z/fxcdP379G
Thank you for the update! It would be nice if the description of #170474 or at least the commit message for 1054a6e mentioned this issue. It's hard to keep track of all commits and guess, which one fixes which issue. Thank you for the fix! |
VPVector(End)PointerRecipes are single-scalar if all their operands are. This should be effectively NFC currently, but it should re-enable cost checking for some more VPWidenMemoryRecipe after
#157387 as discovered by @john-brawn-arm.