Page MenuHomePhabricator

[AMDGPU][GlobalISel] Combine shift + logic + shift with constant operands
ClosedPublic

Authored by mbrkusanin on Tue, Oct 27, 4:41 AM.

Details

Summary

This sequence of instructions can be simplified if they are single use and
some operands are constants. Additional combines may be applied afterwards.

Diff Detail

Event Timeline

mbrkusanin created this revision.Tue, Oct 27, 4:41 AM
mbrkusanin requested review of this revision.Tue, Oct 27, 4:41 AM

This patch is basically a global-isel version of combineShiftOfShiftedLogic() from DAGCombiner.

llvm/test/CodeGen/AMDGPU/GlobalISel/combine-shift-of-shifted-logic.ll
8–9

This can be simplified to a single instruction:
s_lshl_b32 s0, s0, 4
but that will be a different combine.

Same goes for a few more cases below.

  • clang-format
foad added inline comments.Wed, Oct 28, 2:10 AM
llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
244–245

I know you copied this comment from SelectionDAG but I think it really improves latency, not throughput, because the new shifts are independent.

Also you don't mention the other reason for doing this, which is that one of the new shifts might constant-fold away.

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

Also handle G_USHLSAT and G_SSHLSAT?

1666–1669

I don't see why this check is necessary. All we care about is the shift amount, as a uint64_t.

1677–1679

First, it would be simpler to check this by line 1690 where you're already computing the sum. Second, I would expect the > to be >=. Third, you're comparing against BitWidth which is the width of the shift amount, not the value being shifted.

1713

Don't create this. CombinerHelper already has a MIRBuilder.

1720

ShlType is wrong here and below. You want the type of the result, not the type of the shift amount.

1730–1732

It's probably not worth doing this. I think you can rely on dead code elimination.

mbrkusanin marked 5 inline comments as done.
  • Supported G_SSHLSAT and G_USHLSAT and addressed other comments.
  • Added .mir test for G_SSHLSAT and G_USHLSAT.
foad accepted this revision.Mon, Nov 2, 7:02 AM
foad added inline comments.
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
1622

Comment still needs updating.

This revision is now accepted and ready to land.Mon, Nov 2, 7:02 AM
mbrkusanin updated this revision to Diff 302526.Tue, Nov 3, 2:51 AM
  • Updated comments.
arsenm accepted this revision.Tue, Nov 3, 7:42 AM