Skip to content

[TTI][RISCV]Improve costs for whole vector reg extract/insert. #80164

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

Closed
wants to merge 14 commits into from
Closed

[TTI][RISCV]Improve costs for whole vector reg extract/insert. #80164

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
cfd0dcf
[𝘀𝗽𝗿] initial version
alexey-bataev Jan 31, 2024
1689f41
Rebase, address comments
alexey-bataev Jan 31, 2024
e909200
Address comments
alexey-bataev Feb 1, 2024
2dd7afa
Rebase
alexey-bataev Feb 2, 2024
382d5b3
Rebase, Address comments
alexey-bataev Feb 5, 2024
de3be41
Rebase, Address comments
alexey-bataev Feb 12, 2024
51da92f
Rebase, add scalable vectors
alexey-bataev Feb 15, 2024
0ac7359
Rebase, address comments
alexey-bataev Feb 16, 2024
7afc02a
Rebase, address comments
alexey-bataev Feb 19, 2024
93c0d1c
Rebase, address comments
alexey-bataev Feb 20, 2024
f17263c
Rebase, address comments
alexey-bataev Feb 21, 2024
07713b8
Rebase
alexey-bataev Feb 22, 2024
4647802
Rebase
alexey-bataev Feb 28, 2024
a0d744b
Rebase, Address comments
alexey-bataev Feb 29, 2024
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
18 changes: 17 additions & 1 deletion llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,12 +457,28 @@ InstructionCost RISCVTTIImpl::getShuffleCost(TTI::ShuffleKind Kind,
// vslidedown.vi v8, v9, 2
return LT.first *
getRISCVInstructionCost(RISCV::VSLIDEDOWN_VI, LT.second, CostKind);
case TTI::SK_InsertSubvector:
case TTI::SK_InsertSubvector: {
if (Index == 0 && !Args.empty() && any_of(Args, UndefValue::classof))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I tried to split off this piece - or more accurately something vaguely related - and stumbled into something interesting.

The InsertSubvector w/Index=0 is unreachable from everywhere except SLP. TTI::getInstructionCost contains a check for the identity shuffle and always returns 0. improveShuffleKindFromMask will recognize the insert into passthru case as a select (correctly), and thus it doesn't hit this case either. Put together, this means that the index=0 case never makes it from the backend, and thus we have no test coverage via cost model tests.

SLP hits a slightly different codepath here and directly calls getShuffleCost with a possible identity mask. It still can't hit the select case, but it can hit the insert into poison case. SLP appears to have a bunch of guards for this already in various cases.

I'm not really a fan of having untestable logic here. Anyone have any ideas how we can rework this API to ensure SLP can't reach a case which is untestable via costmodel tests?

Copy link
Member Author

Choose a reason for hiding this comment

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

Do not tests for llvm.vector.insert intrinsics check this?

return TTI::TCC_Free;
const unsigned MinVLen = ST->getRealMinVLen();
const unsigned MaxVLen = ST->getRealMaxVLen();
Comment on lines +463 to +464
Copy link
Contributor

Choose a reason for hiding this comment

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

You can use ST->getRealVLen() which was added recently

// Whole vector insert - just the vector itself.
if (auto *FSubTy = dyn_cast<FixedVectorType>(SubTp);
FSubTy && Index == 0 && MinVLen == MaxVLen) {
unsigned TpRegs = getRegUsageForType(Tp);
unsigned SubTpRegs = getRegUsageForType(SubTp);
unsigned NextSubTpRegs = getRegUsageForType(FixedVectorType::get(
Tp->getElementType(), FSubTy->getNumElements() + 1));
if (SubTpRegs != 0 && SubTpRegs != NextSubTpRegs && TpRegs >= SubTpRegs)
Comment on lines +468 to +472
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible for TpRegs < SubTpRegs?

return TTI::TCC_Free;
}

// Example sequence:
// vsetivli zero, 4, e8, mf2, tu, ma (ignored)
// vslideup.vi v8, v9, 2
return LT.first *
getRISCVInstructionCost(RISCV::VSLIDEUP_VI, LT.second, CostKind);
}
case TTI::SK_Select: {
// Example sequence:
// li a0, 90
Expand Down
Loading