This is an archive of the discontinued LLVM Phabricator instance.

[SDAG] Convert FSHL <--> FSHR if the target only supports one of them
ClosedPublic

Authored by foad on Aug 24 2020, 2:57 AM.

Details

Summary

D77152 tried to do this but got it wrong in the shift-by-zero case.
D86430 reverted the wrong code. Reimplement the optimization with
different code depending on whether the shift amount is known to be
non-zero (modulo bitwidth).

This improves code quality for fshl tests on AMDGPU, which only has an
fshr instruction.

Diff Detail

Event Timeline

foad created this revision.Aug 24 2020, 2:57 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 24 2020, 2:57 AM
foad requested review of this revision.Aug 24 2020, 2:57 AM
bjope added inline comments.Aug 24 2020, 8:56 AM
llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
6181

I find these comments a bit confusing, as the full transform is

// fshl X, Y, Z -> fshr (srl X, 1), (fshr (X, Y, 1), ~Z
// fshr X, Y, Z -> fshl (fshl X, Y, 1), (shl (Y, 1), ~Z

I suggest that we either show the full transform like above, or for example add some primes to X and Y on the right hand side to show that it isn't the same X and Y as on the left hand side:

// fshl X, Y, Z -> fshr X', Y', ~Z
// fshr X, Y, Z -> fshl X', Y', ~Z
bjope accepted this revision.Aug 24 2020, 9:00 AM

LGTM (except the earlier nit about the comments with different X and Y values on left and right hand side)

This revision is now accepted and ready to land.Aug 24 2020, 9:00 AM
This revision was landed with ongoing or failed builds.Aug 24 2020, 9:48 AM
This revision was automatically updated to reflect the committed changes.