Skip to content

[LoopVectorizer][AArch64] Add support for partial reduce subtraction #123636

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.

Sign up for GitHub

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

Merged
merged 9 commits into from
Feb 13, 2025
Merged

[LoopVectorizer][AArch64] Add support for partial reduce subtraction #123636

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
8569c7b
[LoopVectorizer][AArch64] Add support for partial reduce subtraction
NickGuy-Arm Jan 28, 2025
2690434
Add more test cases and address comments
NickGuy-Arm Jan 29, 2025
3d7caf1
Match VPValues instead of underlying Values
NickGuy-Arm Jan 31, 2025
132f245
Revert unrelated test changes
NickGuy-Arm Jan 31, 2025
ad49166
Extract the negating subtraction to its own recipe
NickGuy-Arm Feb 7, 2025
3757dd5
Remove cast
NickGuy-Arm Feb 7, 2025
ada0d22
Post-rebase format
NickGuy-Arm Feb 10, 2025
049bf68
Revert change caught by rebase
NickGuy-Arm Feb 11, 2025
842e5d4
Substitute insertion block check with an assert
NickGuy-Arm Feb 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4683,7 +4683,9 @@ InstructionCost AArch64TTIImpl::getPartialReductionCost(
InstructionCost Invalid = InstructionCost::getInvalid();
InstructionCost Cost(TTI::TCC_Basic);

if (Opcode != Instruction::Add)
// Sub opcodes currently only occur in chained cases.
// Independent partial reduction subtractions are still costed as an add
if (Opcode != Instruction::Add && Opcode != Instruction::Sub)
return Invalid;

if (InputTypeA != InputTypeB)
Expand Down
18 changes: 18 additions & 0 deletions llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8800,6 +8800,10 @@ bool VPRecipeBuilder::getScaledReductions(
return false;

using namespace llvm::PatternMatch;
// Use the side-effect of match to replace BinOp only if the pattern is
// matched, we don't care at this point whether it actually matched.
match(BinOp, m_Neg(m_BinOp(BinOp)));
Copy link
Contributor

@david-arm david-arm Feb 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand how this works. We've already bailed out above if BinOp is not a BinaryOperator, which surely means that it cannot simultaneously be a unary operator, which is what m_Neg represents?

Copy link
Contributor

@david-arm david-arm Feb 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I realise now that m_Neg is actually matching a sub 0, %x operation. Please ignore the noise!


Value *A, *B;
if (!match(BinOp->getOperand(0), m_ZExtOrSExt(m_Value(A))) ||
!match(BinOp->getOperand(1), m_ZExtOrSExt(m_Value(B))))
Expand Down Expand Up @@ -8932,6 +8936,20 @@ VPRecipeBuilder::tryToCreatePartialReduction(Instruction *Reduction,
std::swap(BinOp, Accumulator);

unsigned ReductionOpcode = Reduction->getOpcode();
if (ReductionOpcode == Instruction::Sub) {
VPBasicBlock *ParentBlock = Builder.getInsertBlock();
if (!ParentBlock)
return nullptr;
Copy link
Contributor

@fhahn fhahn Feb 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the insert block ever be null? I'd expect it to always be set at this point?

Copy link
Contributor Author

@NickGuy-Arm NickGuy-Arm Feb 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't seen any case where it would be, so this was just a bit of defensiveness for something I wasn't 100% sure on. I can switch it to an assert, or remove it altogether if you think the check is irrelevant.

Edit: Looks like VPBuilder::clearInsertPoint can result in getInsertPoint returning a nullptr. So I've changed it to an assert.


auto *const Zero = ConstantInt::get(Reduction->getType(), 0);
SmallVector<VPValue *, 2> Ops;
Ops.push_back(Plan.getOrAddLiveIn(Zero));
Ops.push_back(BinOp);
BinOp = new VPWidenRecipe(*Reduction, make_range(Ops.begin(), Ops.end()));
ParentBlock->appendRecipe(BinOp->getDefiningRecipe());
ReductionOpcode = Instruction::Add;
}

if (CM.blockNeedsPredicationForAnyReason(Reduction->getParent())) {
assert((ReductionOpcode == Instruction::Add ||
ReductionOpcode == Instruction::Sub) &&
Expand Down
10 changes: 8 additions & 2 deletions llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include "llvm/IR/Instruction.h"
#include "llvm/IR/Instructions.h"
#include "llvm/IR/Intrinsics.h"
#include "llvm/IR/PatternMatch.h"
#include "llvm/IR/Type.h"
#include "llvm/IR/Value.h"
#include "llvm/IR/VectorBuilder.h"
Expand Down Expand Up @@ -284,13 +285,18 @@ InstructionCost
VPPartialReductionRecipe::computeCost(ElementCount VF,
VPCostContext &Ctx) const {
std::optional<unsigned> Opcode = std::nullopt;
VPRecipeBase *BinOpR = getOperand(0)->getDefiningRecipe();
VPValue *BinOp = getOperand(0);

// If the partial reduction is predicated, a select will be operand 0 rather
// than the binary op
using namespace llvm::VPlanPatternMatch;
if (match(getOperand(0), m_Select(m_VPValue(), m_VPValue(), m_VPValue())))
BinOpR = BinOpR->getOperand(1)->getDefiningRecipe();
BinOp = BinOp->getDefiningRecipe()->getOperand(1);

// If BinOp is a negation, use the side effect of match to assign the actual
// binary operation to BinOp
match(BinOp, m_Binary<Instruction::Sub>(m_SpecificInt(0), m_VPValue(BinOp)));
VPRecipeBase *BinOpR = BinOp->getDefiningRecipe();

if (auto *WidenR = dyn_cast<VPWidenRecipe>(BinOpR))
Opcode = std::make_optional(WidenR->getOpcode());
Expand Down
Loading