[PATCH] D152649: [AMDGPU] Enable Atomic Optimizer by default.
Jay Foad via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 14 07:59:44 PDT 2023
foad accepted this revision.
foad added a comment.
This revision is now accepted and ready to land.
LGTM, with or without changing the default strategy.
================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp:278-281
static cl::opt<bool>
EnableAtomicOptimizations("amdgpu-atomic-optimizations",
cl::desc("Enable atomic optimizations"),
+ cl::init(true), cl::Hidden);
----------------
foad wrote:
> pravinjagtap wrote:
> > foad wrote:
> > > pravinjagtap wrote:
> > > > arsenm wrote:
> > > > > The old option can be dropped, none should be a value for the strategy
> > > > > The old option can be dropped, none should be a value for the strategy
> > > >
> > > > I think, dropping the old option (`amdgpu-atomic-optimizations`) will break LLPC at the moment.
> > > >
> > > > I think right way to do this is, transfer the responsibility of enabling the pass to `amdgpu-atomic-optimizer-strategy` and keep the old option `amdgpu-atomic-optimizations` till LLPC removes all the usage of it.
> > > >
> > > > @foad
> > > Right, please keep it until LLPC has been updated again, then you can remove it.
> > > Right, please keep it until LLPC has been updated again, then you can remove it.
> >
> > So, we will be having three options for strategy { DPP, Iterative, None}. Here `None` will disable the pass (will be used in lit tests) & DPP/Iterative will enable the pass with given strategy.
> >
> > The `amdgpu-atomic-optimizer-strategy` will be defaulted to `Iterative` (not None as suggested by @arsenm ) and then we could drop the old option `amdgpu-atomic-optimizations`.
> >
> > Is this fine or is there any other way of handling this situation ?
> >
> >
> Sounds fine but in this patch please just change the default for amdgpu-atomic-optimizations from false to true. The other option changes can be done in a separate patch.
Sorry I was not clear: It is also fine to change the default strategy in this patch, if you want to. But please do not remove the `amdgpu-atomic-optimizations` option in this patch.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D152649/new/
https://reviews.llvm.org/D152649
More information about the llvm-commits
mailing list