This is an archive of the discontinued LLVM Phabricator instance.

[CostModel] Align the cost model for intrinsics for scalable/fixed-width vectors.
ClosedPublic

Authored by sdesmalen on Feb 25 2021, 6:35 AM.

Details

Summary

Let getIntrinsicInstrCost call getTypeBasedIntrinsicInstrCost for scalable vectors,
similar to how this is done for fixed-width vectors, instead of falling back
on BaseT::getIntrinsicInstrCost().

If the intrinsic cannot be costed (or is not overloaded by the target),
it will return InstructionCost::getInvalid() instead.

Depends on D97469

Diff Detail

Event Timeline

sdesmalen requested review of this revision.Feb 25 2021, 6:35 AM
sdesmalen created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 25 2021, 6:35 AM
dmgreen added inline comments.Feb 28 2021, 10:39 AM
llvm/include/llvm/CodeGen/BasicTTIImpl.h
1345

Should this ScalarizationCost and IntrinsicCostAttributes::ScalarizationCost be converted to InstructionCost too?

sdesmalen added inline comments.Mar 1 2021, 5:33 AM
llvm/include/llvm/CodeGen/BasicTTIImpl.h
1345

Yes, I have another patch for that which updates the interface of getScalarizationOverhead to return InstructionCost at which points this needs to become an InstructionCost as well. I haven't posted this patch yet, but will do so soon.

sdesmalen updated this revision to Diff 327096.Mar 1 2021, 6:21 AM

Rebased patch.

david-arm accepted this revision.Mar 3 2021, 1:51 AM

LGTM!

llvm/include/llvm/CodeGen/BasicTTIImpl.h
1346

nit: Could you just use RetVF.isFixedLengthVector() instead here?

1392

nit: Could you address the formatting issues?

1809

nit: Formatting issues

This revision is now accepted and ready to land.Mar 3 2021, 1:51 AM
This revision was landed with ongoing or failed builds.Mar 31 2021, 7:02 AM
This revision was automatically updated to reflect the committed changes.
sdesmalen marked 4 inline comments as done.

Thanks for the review comments!

llvm/include/llvm/CodeGen/BasicTTIImpl.h
1346

That's a method of EVT, not ElementCount unfortunately.