This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Do not generate mul with 1 in AMDGPU Atomic Optimizer
ClosedPublic

Authored by mbrkusanin on Sep 25 2020, 9:10 AM.

Details

Summary

Check if operand of mul is constant value of one for certain atomic
instructions in order to avoid making unnecessary instructions when
-amdgpu-atomic-optimizer is present.

Diff Detail

Event Timeline

mbrkusanin created this revision.Sep 25 2020, 9:10 AM
mbrkusanin requested review of this revision.Sep 25 2020, 9:10 AM
arsenm added inline comments.Sep 25 2020, 10:55 AM
llvm/test/CodeGen/AMDGPU/atomic_optimizations_mul_one.mir
1

It's weird to use MIR and -run-pass for an IR pass. Should have regular ll test, with an end to end llc line and an opt line for just the pass

foad added inline comments.Sep 28 2020, 4:16 AM
llvm/lib/Target/AMDGPU/AMDGPUAtomicOptimizer.cpp
407

It would be neater to have a BuildMul helper function that checks for and optimizes the mul-by-1 case.

  • Changed test to opt + llc global-isel (SDag was already removing these mul instructions, just later on). Should I split the file into two tests?
mbrkusanin added inline comments.Sep 28 2020, 7:50 AM
llvm/lib/Target/AMDGPU/AMDGPUAtomicOptimizer.cpp
407

A new BuildMul or update the current one (looks like some unittests need to be updated in this case)?

foad added inline comments.Sep 28 2020, 7:56 AM
llvm/lib/Target/AMDGPU/AMDGPUAtomicOptimizer.cpp
407

I was suggesting a new BuildMul helper function in this source file, not changing IRBuilder at all.

  • Added buildMul that checks for mul with 1.
arsenm added inline comments.Sep 29 2020, 6:31 AM
llvm/test/CodeGen/AMDGPU/GlobalISel/atomic_optimizations_mul_one.ll
7–10 ↗(On Diff #294714)

These negative checks are extremely fragile. These should use positive checks

  • Updated llc test check lines.
llvm/test/CodeGen/AMDGPU/GlobalISel/atomic_optimizations_mul_one.ll
7–10 ↗(On Diff #294714)

Just llc or opt check lines as well?

arsenm added inline comments.Sep 29 2020, 7:22 AM
llvm/test/CodeGen/AMDGPU/GlobalISel/atomic_optimizations_mul_one.ll
7–10 ↗(On Diff #294714)

Both. Negative checks are just too easy to get wrong

  • Updated opt test check lines.
arsenm accepted this revision.Sep 29 2020, 7:46 AM
This revision is now accepted and ready to land.Sep 29 2020, 7:46 AM