This is an archive of the discontinued LLVM Phabricator instance.

[AArch64][GlobalISel] combine and + [la]sr => ubfx
ClosedPublic

Authored by jroelofs on Oct 14 2021, 2:22 PM.

Diff Detail

Event Timeline

jroelofs created this revision.Oct 14 2021, 2:22 PM
jroelofs requested review of this revision.Oct 14 2021, 2:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 14 2021, 2:22 PM
jroelofs edited the summary of this revision. (Show Details)Oct 14 2021, 5:13 PM
This revision is now accepted and ready to land.Oct 14 2021, 6:21 PM
foad requested changes to this revision.Oct 15 2021, 2:15 AM

I don't think there are any cases where it is beneficial to combine ashr (and x, n), k -> sbfx (are there?), because for any case where you can do that, it would be better just to remove the and and leave it as an ashr.

This revision now requires changes to proceed.Oct 15 2021, 2:15 AM
foad added a comment.Oct 15 2021, 2:17 AM

it would be better just to remove the and and leave it as an ashr.

... and that should be done by a general demanded bits analysis, which could spot that the and is redundant.

I don't think there are any cases where it is beneficial to combine ashr (and x, n), k -> sbfx (are there?), because for any case where you can do that, it would be better just to remove the and and leave it as an ashr.

Great point! Thanks for catching that.

jroelofs updated this revision to Diff 380051.Oct 15 2021, 10:14 AM
jroelofs retitled this revision from [AArch64][GlobalISel] combine and + [la]sr => [su]bfx to [AArch64][GlobalISel] combine and + [la]sr => ubfx.
foad accepted this revision.Oct 18 2021, 12:38 AM

LGTM modulo nits, thanks.

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
4138

No need for this variable.

4157

Just "ubfx".

This revision is now accepted and ready to land.Oct 18 2021, 12:38 AM
This revision was automatically updated to reflect the committed changes.