This is an archive of the discontinued LLVM Phabricator instance.

[AARCH64][COST] Improve cost of reverse shuffles for AArch64
ClosedPublic

Authored by Miss_Grape on Aug 26 2022, 3:58 AM.

Diff Detail

Event Timeline

Miss_Grape created this revision.Aug 26 2022, 3:58 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 26 2022, 3:58 AM
Miss_Grape requested review of this revision.Aug 26 2022, 3:58 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 26 2022, 3:58 AM
Miss_Grape added inline comments.
llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
2979

Why not set the scene for N X Ty < 64?
eg: {TTI::SK_Reverse, MVT::v2i16, 1}
@david Greene

dmgreen added inline comments.
llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
2979

Hello. I don't have that many e's in my name, but I presume you meant me :)

This table acts upon legal types, through LT.second in the call to CostTableLookup. That means a v2i16 will be treated as a v2i32 (extending the i16's into the upper halves of each of the i32's), and should see a low cost of 1 because of it.

If you think the costs are wrong and need adjusting, they might need to be handled separately from this table.

Miss_Grape marked an inline comment as done.Aug 29 2022, 7:49 PM
dmgreen accepted this revision.Sep 6 2022, 1:03 AM

Hello - the updates for the comments and the new tests sound OK. I'm not sure if I'm misunderstanding and there is something else you want to propose, but these changes LGTM.

This revision is now accepted and ready to land.Sep 6 2022, 1:03 AM
This revision was landed with ongoing or failed builds.Sep 8 2022, 3:56 AM
This revision was automatically updated to reflect the committed changes.

This should probably have had NFC in the title, and ended up being quite different to the version that was reviewed. We have tried not to format the tables in the past because it just creates a worse table (and obscures git-blame). I've tried to clean up the code again a little in 3875c38adf40.