This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Switch to the new cl option amdgpu-atomic-optimizer-strategy.
ClosedPublic

Authored by pravinjagtap on Jun 15 2023, 3:37 AM.

Details

Summary

Atomic optimizer is turned on by default through D152649. This patch
removes the usage of old command line option amdgpu-atomic-optimizations
and transfer the responsibility to amdgpu-atomic-optimizer-strategy.

We can safely remove old option when LLPC remove its all usage.

Diff Detail

Event Timeline

pravinjagtap created this revision.Jun 15 2023, 3:37 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 15 2023, 3:37 AM
pravinjagtap requested review of this revision.Jun 15 2023, 3:37 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 15 2023, 3:37 AM
pravinjagtap edited the summary of this revision. (Show Details)Jun 15 2023, 3:39 AM
pravinjagtap added reviewers: foad, arsenm, Restricted Project.
cdevadas added inline comments.Jun 15 2023, 4:41 AM
llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
278

clang-format

clang-format & rebase

pravinjagtap marked an inline comment as done.Jun 16 2023, 2:56 AM
cdevadas accepted this revision.Jun 16 2023, 6:47 AM

LGTM.
Wait for a day if others have any comments.

This revision is now accepted and ready to land.Jun 16 2023, 6:47 AM

Ping @foad.
I think, we can remove all the usage of old command line option (-amdgpu-atomic-optimizations) in this patch since LLPC is no longer using it.

foad accepted this revision.Jun 21 2023, 2:26 AM

Ping @foad.
I think, we can remove all the usage of old command line option (-amdgpu-atomic-optimizations) in this patch since LLPC is no longer using it.

Yup.

Removed all usage of amdgpu-atomic-optimizations

arsenm added inline comments.Jun 21 2023, 3:43 AM
llvm/lib/Target/AMDGPU/AMDGPUAtomicOptimizer.cpp
164

Just early return, running the pass standalone should work

871

So the pass behavior with none is to just crash?

pravinjagtap added inline comments.Jun 21 2023, 4:35 AM
llvm/lib/Target/AMDGPU/AMDGPUAtomicOptimizer.cpp
871

Execution will not reach here for None strategy.

arsenm added inline comments.Jun 21 2023, 5:14 PM
llvm/lib/Target/AMDGPU/AMDGPUAtomicOptimizer.cpp
871

So then there's no point in changing it? either leave it or at else llvm_unreachable

added llvm_unreachable

arsenm accepted this revision.Jun 22 2023, 2:53 AM
pravinjagtap edited the summary of this revision. (Show Details)Jun 22 2023, 3:47 AM