This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Add a special case for shifting by (BitWidth - 1) - X
ClosedPublic

Authored by foad on Apr 2 2020, 9:15 AM.

Diff Detail

Event Timeline

foad created this revision.Apr 2 2020, 9:15 AM

Commit the new tests against trunk and then rebase to show the codegen diffs

foad updated this revision to Diff 264445.May 16 2020, 9:12 AM

Rebase after precommitting tests.

What happens if the shift amount has more than one use?

llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
2794

Please use mvn to NOT a register. (This doesn't affect performance, but it's more readable.)

foad updated this revision to Diff 407091.Feb 9 2022, 1:56 AM

Rebase. Generate mvn instead of eon.

foad marked an inline comment as done.Feb 9 2022, 1:58 AM

What happens if the shift amount has more than one use?

Not sure, but the same as the existing special case for shifting by BitWidth - X.

What happens if the shift amount has more than one use?

Not sure, but the same as the existing special case for shifting by BitWidth - X.

So, we emit the shift amount separately. Which isn't a big deal, I guess, but we should have a testcase. Something like:

define void @foo(i64* %valptr, i64* %shamtptr, i64 %shamt) {
  %val = load i64, i64* %valptr
  %negshamt = sub i64 64, %shamt
  %shifted = ashr i64 %val, %negshamt
  store i64 %shifted, i64* %valptr
  store i64 %negshamt, i64* %shamtptr
  ret void
}
foad updated this revision to Diff 407448.Feb 10 2022, 2:53 AM

Rebase on multi-use tests.

So, we emit the shift amount separately.

So it looks like we should only do this when the shift amount has a single use? Can I leave that for a separate patch?

efriedma accepted this revision.Feb 10 2022, 10:43 AM

LGTM. I'm fine putting the multi-use handling in a followup. (For that, I guess you might also want to consider the case where a shift amount is used by multiple shifts...)

This revision is now accepted and ready to land.Feb 10 2022, 10:43 AM
This revision was landed with ongoing or failed builds.Feb 11 2022, 12:27 AM
This revision was automatically updated to reflect the committed changes.