This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Drop _oneuse checks from med3 patterns
ClosedPublic

Authored by bogner on Aug 24 2022, 4:35 PM.

Details

Summary

We use _oneuse checks to make sure combines won't accidentally increase code size, but this prevents the optimization in cases where we happen to want to clamp multiple values to the same range

It's safe to drop these checks for two reasons:

  1. The pattern of max/min operations for med3 is complicated enough it's unlikely to come up by accident, so this will still only fire when appropriate to do so
  2. Even if every intermediate is used and we don't save a single operation, we still won't end up with more operations since the med3 replaces the final max/min.

In pathological cases we could potentially end up with a larger encoding size or possibly slightly increased vgpr pressure, but the risk of that is low, especially considering the upside.

Diff Detail

Event Timeline

bogner created this revision.Aug 24 2022, 4:35 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 24 2022, 4:35 PM
bogner requested review of this revision.Aug 24 2022, 4:35 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 24 2022, 4:35 PM
bogner edited the summary of this revision. (Show Details)Aug 24 2022, 4:35 PM
bogner updated this revision to Diff 455434.Aug 24 2022, 5:20 PM

Fixed a typo in the CHECK lines

foad added inline comments.Aug 30 2022, 3:40 AM
llvm/test/CodeGen/AMDGPU/GlobalISel/inst-select-pattern-smed3.mir
148

Please precommit this new test and rebase so that the patch shows the codegen diff.

llvm/test/CodeGen/AMDGPU/GlobalISel/inst-select-pattern-umed3.mir
148

Please precommit this new test and rebase so that the patch shows the codegen diff.

bogner updated this revision to Diff 456744.Aug 30 2022, 12:08 PM

Rebased after pre-committing generated tests

bogner marked 2 inline comments as done.Aug 30 2022, 12:09 PM
foad accepted this revision.Sep 1 2022, 2:13 AM

LGTM, thanks! Please use full context (e.g. with -U9999) when uploading diffs.

This revision is now accepted and ready to land.Sep 1 2022, 2:13 AM
This revision was automatically updated to reflect the committed changes.