This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Don't use tbl lowering if ZExt can be folded into user.
ClosedPublic

Authored by fhahn on May 12 2023, 2:05 PM.

Details

Summary

If the ZExt can be lowered to a single ZExt to the next power-of-2 and
the remaining ZExt folded into the user, don't use tbl lowering.

Fixes #62620.

Diff Detail

Unit TestsFailed

Event Timeline

fhahn created this revision.May 12 2023, 2:05 PM
fhahn requested review of this revision.May 12 2023, 2:05 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 12 2023, 2:05 PM
efriedma added inline comments.May 14 2023, 1:06 PM
llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
1818

Does this code do the right thing when the "Args" includes a cast with the wrong source type? (In this case, the source type is i16, but the type of the operand of the cast would actually be i8.)

fhahn updated this revision to Diff 523518.May 18 2023, 12:51 PM

Generalize code to properly handling mull case with casts with different source types, thanks!

fhahn marked an inline comment as done.May 18 2023, 12:53 PM
fhahn added inline comments.
llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
1818

I don't think so, thanks! I updated the code to pass both source types in that case, not sure if there's a better way of handling that.

fhahn marked an inline comment as done.May 30 2023, 8:56 AM

ping :)

efriedma accepted this revision.Jun 1 2023, 11:09 AM

LGTM

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
14422

Maybe just explicitly check "DstWidth != 16 && DstWidth != 32"?

This revision is now accepted and ready to land.Jun 1 2023, 11:09 AM
This revision was landed with ongoing or failed builds.Jun 2 2023, 3:53 AM
This revision was automatically updated to reflect the committed changes.
fhahn marked an inline comment as done.Jun 2 2023, 4:51 AM
fhahn added inline comments.
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
14422

The current codegen also works for like i24 and there are existing tests for that; I left the check as is for now to avoid changing behavior unrelated to the patch

v01dXYZ added inline comments.
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
14622

Sorry the question is dumb but why does it use TruncType instead of DoubleSrcType := SrcType with Element type width multiplied by 2 ?

14629

This assignment is not necessary.

llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
1869

There is a typo (I think).

fhahn marked 4 inline comments as done.Jun 15 2023, 8:38 AM

Thanks for taking a look. Comments should be addressed in baebe719a52acbe0a5aaa04dcf1abcfd4035bf1f and response inline.

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
14622

TBL lowering is not profitable if only one zext step is saved. If the zext to the next smaller type is free (i.e. it can be folded into the user), then only 1 step will be needed and no tbl should be generated

14629

Thanks, removed!

llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
1869

Yes, this should check I's source type, should be fixed.